On Wed, May 18, 2005 at 01:14:07AM +0300, Michael S. Tsirkin wrote:
> Quoting r. Libor Michalek <[EMAIL PROTECTED]>:
> > Subject: Re: [PATCH] (repost) sdp: replace mlock with get_user_pages
> >
> > On Sun, May 15, 2005 at 11:05:03AM +0300, Michael S. Tsirkin wrote:
> > > Quoting r. Libor Michalek <[EMAIL PROTECTED]>:
> >
> > > > + /*
> > > > + * valid result can be 0 or 1 for complete so
> > > > + * we ignore the value.
> > > > + */
> > > > + (void)aio_complete(iocb->req, value, 0);
> > > >
> > > > if (in_atomic() || irqs_disabled()) {
> > > > INIT_WORK(&iocb->completion, do_iocb_complete, (void
> > > > *)iocb);
> > >
> > > By the way, as was already discussed here, in_atomic() may not
> > > catch all cases where its illegal to sleep.
> > > If the iocb functions get passed the socket parameter,
> > > the iocb can use the conn->lock.users flag to figure out that its
> > > OK to sleep.
> >
> > The conn->lock.users flag is not appropriate since it can be set and
> > it's still OK to sleep. For example it is set in numerous places where
> > we call kmalloc with the GFP_KERNEL flag. The conn->lock.users flag is
> > only modified inside a spinlock with IRQs disabled, but it can be either
> > value outside of that spinlock.
> >
> > -Libor
> >
>
> SDP_CONN_LOCK_IRQ does not set the users flag.
>
> If users is set this means that the socket was locked with
> sdp_conn_lock, which in turn means we are not in interrupt.
> Isnt that right?
Now I see what you're driving at.
Currently, the flag is still set when we process the backlog in
sdp_conn_unlock() and then sdp_conn_internal_unlock() which is handled
under the spinlock, but there's no reason I can see not to clear it
ahead of sdp_conn_internal_unlock() instead of after, since it's all
under the same spinlock. Same goes for sdp_conn_relock() it would
need to be cleared/reset before/after the drain all within the same
spinlock. At that point I see no reason not to use it to determine if
it is safe to sleep. Either that or make a second flag to avoid
overloading the "users" flag's meaning.
However, I don't think having this knowledge in iocb_complete()
fixes the race condition I described. In the race I saw the iocb_complete()
was called correctly inside of sdp_conn_unlock() and then a following
aio write finished synchronously.
-Libor
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
