Re: [Xen-devel] [PATCH v4 3/6] xen: add process_pending_softirqs_norcu() for keyhandlers

[Jürgen Groß <jgross@xxxxxxxx>]

On 11.03.20 10:36, Jan Beulich wrote:
> On 11.03.2020 10:27, Jürgen Groß wrote:
> > On 11.03.20 10:25, Jan Beulich wrote:
> > > On 11.03.2020 07:07, Jürgen Groß wrote:
> > > > On 10.03.20 18:02, Jan Beulich wrote:
> > > > > On 10.03.2020 08:28, Juergen Gross wrote:
> > > > > > --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> > > > > > +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> > > > > > @@ -587,7 +587,7 @@ static void amd_dump_p2m_table_level(struct page_info* pg, 
> > > > > > int level,
> > > > > >             struct amd_iommu_pte *pde = &table_vaddr[index];
> > > > > >     
> > > > > >             if ( !(index % 2) )
> > > > > > -            process_pending_softirqs();
> > > > > > +            process_pending_softirqs_norcu();
> > > > > At the example of this - the property of holding an RCU lock is
> > > > > entirely invisible here, as it's the generic
> > > > > iommu_dump_p2m_table() which acquires it. This suggest to me that
> > > > > going forward breaking this is going to be very likely. Couldn't
> > > > > process_pending_softirqs() exclude RCU handling when finding
> > > > > preempt_count() to return non-zero?
> > > > This can be done, but then the non-debug build would require to have
> > > > non-empty rcu lock functions.
> > > I guess I don't understand - I see only one version of them:
> > >
> > > #define rcu_read_lock(x)       ({ ((void)(x)); preempt_disable(); })
> > > #define rcu_read_unlock(x)     ({ ((void)(x)); preempt_enable(); })
> > >
> > > Same for the preempt count adjustment operations.
> > See patch 5.
> Which I haven't looked at yet, and which I also shouldn't need to
> look at to understand the patch here. If this is a preparatory
> change rather than some form of fix or improvement, then the
> description should say so.
This was just meant as an answer to your question regarding considering
preempt_count(). Just changing this patch here and then undoing the
change again in patch 5 is no option IMO, so I gave a hint why using
preempt_count() might be a bad idea.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel