TAKE 981498 - Use KM_NOFS for debug trace buffers

TAKE 981498 - Use KM_NOFS for debug trace buffers

[Lachlan McIlroy]

Use KM_NOFS for debug trace buffers

Use KM_NOFS to prevent recursion back into the filesystem which can
cause deadlocks.

In the case of xfs_iread() we hold the lock on the inode cluster buffer
while allocating memory for the trace buffers.  If we recurse back into
XFS to flush data that may require a transaction to allocate extents
which needs log space.  This can deadlock with the xfsaild thread which
can't push the tail of the log because it is trying to get the inode
cluster buffer lock.

Date:  Wed Aug  6 16:15:14 AEST 2008
Workarea:  redback.melbourne.sgi.com:/home/lachlan/isms/2.6.x-mm
Inspected by:  david@fromorbit.com
Author:  lachlan

The following file(s) were checked into:
  longdrop.melbourne.sgi.com:/isms/linux/2.6.x-xfs-melb


Modid:  xfs-linux-melb:xfs-kern:31838a
fs/xfs/xfs_log.c - 1.362 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_log.c.diff?r1=text&tr1=1.362&r2=text&tr2=1.361&f=h
fs/xfs/xfs_buf_item.c - 1.168 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_buf_item.c.diff?r1=text&tr1=1.168&r2=text&tr2=1.167&f=h
fs/xfs/xfs_inode.c - 1.518 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_inode.c.diff?r1=text&tr1=1.518&r2=text&tr2=1.517&f=h
fs/xfs/quota/xfs_dquot.c - 1.38 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/quota/xfs_dquot.c.diff?r1=text&tr1=1.38&r2=text&tr2=1.37&f=h
fs/xfs/linux-2.6/xfs_buf.c - 1.262 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/linux-2.6/xfs_buf.c.diff?r1=text&tr1=1.262&r2=text&tr2=1.261&f=h
fs/xfs/xfs_filestream.c - 1.9 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_filestream.c.diff?r1=text&tr1=1.9&r2=text&tr2=1.8&f=h
	- Use KM_NOFS for debug trace buffers

Re: TAKE 981498 - Use KM_NOFS for debug trace buffers

[Bhagi rathi]

I couldn't get a chance to read the diff's completely. If I click on
Lachlan's url for diff's, I couldn't access them. It looks to me that
the issue is not just with trace buffers. It can extend to xfs_iformat
as well. The same dead-lock can spring via

xfs_iread -> xfs_iformat -> xfs_iformat_extents -> xfs_iext_add ->
xfs_iext_inline_to_direct -> which can do kmem_alloc with
KM_SLEEP flag.


The source of the problem is that holding a lock and entering into
file-system once again. This can lead to dead-lock on the same
clustered buffer during cleaning of log space.

Cheers,
Bhagi.


On Wed, Aug 6, 2008 at 11:45 AM, Lachlan McIlroy <lachlan@sgi.com> wrote:

> Use KM_NOFS for debug trace buffers
>
> Use KM_NOFS to prevent recursion back into the filesystem which can
> cause deadlocks.
>
> In the case of xfs_iread() we hold the lock on the inode cluster buffer
> while allocating memory for the trace buffers.  If we recurse back into
> XFS to flush data that may require a transaction to allocate extents
> which needs log space.  This can deadlock with the xfsaild thread which
> can't push the tail of the log because it is trying to get the inode
> cluster buffer lock.
>
> Date:  Wed Aug  6 16:15:14 AEST 2008
> Workarea:  redback.melbourne.sgi.com:/home/lachlan/isms/2.6.x-mm
> Inspected by:  david@fromorbit.com
> Author:  lachlan
>
> The following file(s) were checked into:
>  longdrop.melbourne.sgi.com:/isms/linux/2.6.x-xfs-melb
>
>
> Modid:  xfs-linux-melb:xfs-kern:31838a
> fs/xfs/xfs_log.c - 1.362 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/> xfs_log.c.diff?r1=text&tr1=1.362&r2=text&tr2=1.361&f=h
>
> http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_log.c.diff?r1=text&tr1=1.362&r2=text&tr2=1.361&f=h
> fs/xfs/xfs_buf_item.c<http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_log.c.diff?r1=text&tr1=1.362&r2=text&tr2=1.361&f=hfs/xfs/xfs_buf_item.c>- 1.168 - changed
>
> http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_buf_item.c.diff?r1=text&tr1=1.168&r2=text&tr2=1.167&f=h
> fs/xfs/xfs_inode.c<http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_buf_item.c.diff?r1=text&tr1=1.168&r2=text&tr2=1.167&f=hfs/xfs/xfs_inode.c>- 1.518 - changed
>
> http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_inode.c.diff?r1=text&tr1=1.518&r2=text&tr2=1.517&f=h
> fs/xfs/quota/xfs_dquot.c<http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_inode.c.diff?r1=text&tr1=1.518&r2=text&tr2=1.517&f=hfs/xfs/quota/xfs_dquot.c>- 1.38 - changed
>
> http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/quota/xfs_dquot.c.diff?r1=text&tr1=1.38&r2=text&tr2=1.37&f=h
> fs/xfs/linux-2.6/xfs_buf.c<http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/quota/xfs_dquot.c.diff?r1=text&tr1=1.38&r2=text&tr2=1.37&f=hfs/xfs/linux-2.6/xfs_buf.c>- 1.262 - changed
>
> http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/linux-2.6/xfs_buf.c.diff?r1=text&tr1=1.262&r2=text&tr2=1.261&f=h
> fs/xfs/xfs_filestream.c<http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/linux-2.6/xfs_buf.c.diff?r1=text&tr1=1.262&r2=text&tr2=1.261&f=hfs/xfs/xfs_filestream.c>- 1.9 - changed
>
> http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_filestream.c.diff?r1=text&tr1=1.9&r2=text&tr2=1.8&f=h
>        - Use KM_NOFS for debug trace buffers
>
>
>
>
>


[[HTML alternate version deleted]]

Re: TAKE 981498 - Use KM_NOFS for debug trace buffers

[Eric Sandeen]

Bhagi rathi wrote:
> I couldn't get a chance to read the diff's completely. If I click on
> Lachlan's url for diff's, I couldn't access them. 

Try again, it takes a while for cvs to catch up.

-Eric

> It looks to me that
> the issue is not just with trace buffers. It can extend to xfs_iformat
> as well. The same dead-lock can spring via
> 
> xfs_iread -> xfs_iformat -> xfs_iformat_extents -> xfs_iext_add ->
> xfs_iext_inline_to_direct -> which can do kmem_alloc with
> KM_SLEEP flag.
> 
> 
> The source of the problem is that holding a lock and entering into
> file-system once again. This can lead to dead-lock on the same
> clustered buffer during cleaning of log space.
> 
> Cheers,
> Bhagi.

Re: TAKE 981498 - Use KM_NOFS for debug trace buffers

[Dave Chinner]

On Wed, Aug 06, 2008 at 10:42:15PM +0530, Bhagi rathi wrote:
> I couldn't get a chance to read the diff's completely. If I click on
> Lachlan's url for diff's, I couldn't access them. It looks to me that
> the issue is not just with trace buffers. It can extend to xfs_iformat
> as well. The same dead-lock can spring via
> 
> xfs_iread -> xfs_iformat -> xfs_iformat_extents -> xfs_iext_add ->
> xfs_iext_inline_to_direct -> which can do kmem_alloc with
> KM_SLEEP flag.

Fixed already:


Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: TAKE 981498 - Use KM_NOFS for debug trace buffers

[Dave Chinner]

On Thu, Aug 07, 2008 at 06:19:57AM +1000, Dave Chinner wrote:
> On Wed, Aug 06, 2008 at 10:42:15PM +0530, Bhagi rathi wrote:
> > I couldn't get a chance to read the diff's completely. If I click on
> > Lachlan's url for diff's, I couldn't access them. It looks to me that
> > the issue is not just with trace buffers. It can extend to xfs_iformat
> > as well. The same dead-lock can spring via
> > 
> > xfs_iread -> xfs_iformat -> xfs_iformat_extents -> xfs_iext_add ->
> > xfs_iext_inline_to_direct -> which can do kmem_alloc with
> > KM_SLEEP flag.
> 
> Fixed already:
> 
> 

Hmmm. where did that url go? Try again:


Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: TAKE 981498 - Use KM_NOFS for debug trace buffers

[Dave Chinner]

On Thu, Aug 07, 2008 at 06:27:46AM +1000, Dave Chinner wrote:
> On Thu, Aug 07, 2008 at 06:19:57AM +1000, Dave Chinner wrote:
> > On Wed, Aug 06, 2008 at 10:42:15PM +0530, Bhagi rathi wrote:
> > > I couldn't get a chance to read the diff's completely. If I click on
> > > Lachlan's url for diff's, I couldn't access them. It looks to me that
> > > the issue is not just with trace buffers. It can extend to xfs_iformat
> > > as well. The same dead-lock can spring via
> > > 
> > > xfs_iread -> xfs_iformat -> xfs_iformat_extents -> xfs_iext_add ->
> > > xfs_iext_inline_to_direct -> which can do kmem_alloc with
> > > KM_SLEEP flag.
> > 
> > Fixed already:
> > 
> > 
> 
> Hmmm. where did that url go? Try again:

Ok, something is stripping URLs out of emails. I just sent that URL
to myself and it wasn't stripped so it's not my mail infrastructure
that is doing it.

Did someone "upgrade" the spam filters on oss.sgi.com or the
barracuda overnight?

The link - minus the "http://" bit is:

oss.sgi.com/cgi-bin/gitweb.cgi?p=xfs/xfs-2.6.git;a=commit;h=8c6266658cb76e282c14cb92f8ba5a1c674f4928

let's see if that gets stripped....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: TAKE 981498 - Use KM_NOFS for debug trace buffers

[Russell Cattelan]

Dave Chinner wrote:
> On Thu, Aug 07, 2008 at 06:27:46AM +1000, Dave Chinner wrote:
>   
>> On Thu, Aug 07, 2008 at 06:19:57AM +1000, Dave Chinner wrote:
>>     
>>> On Wed, Aug 06, 2008 at 10:42:15PM +0530, Bhagi rathi wrote:
>>>       
>>>> I couldn't get a chance to read the diff's completely. If I click on
>>>> Lachlan's url for diff's, I couldn't access them. It looks to me that
>>>> the issue is not just with trace buffers. It can extend to xfs_iformat
>>>> as well. The same dead-lock can spring via
>>>>
>>>> xfs_iread -> xfs_iformat -> xfs_iformat_extents -> xfs_iext_add ->
>>>> xfs_iext_inline_to_direct -> which can do kmem_alloc with
>>>> KM_SLEEP flag.
>>>>         
>>> Fixed already:
>>>
>>>
>>>       
>> Hmmm. where did that url go? Try again:
>>     
>
> Ok, something is stripping URLs out of emails. I just sent that URL
> to myself and it wasn't stripped so it's not my mail infrastructure
> that is doing it.
>
> Did someone "upgrade" the spam filters on oss.sgi.com or the
> barracuda overnight?
>   
not oss
I'm seeing the correct url's BTW
> The link - minus the "http://" bit is:
>
> oss.sgi.com/cgi-bin/gitweb.cgi?p=xfs/xfs-2.6.git;a=commit;h=8c6266658cb76e282c14cb92f8ba5a1c674f4928
>
> let's see if that gets stripped....
>
> Cheers,
>
> Dave.
>   

Re: TAKE 981498 - Use KM_NOFS for debug trace buffers

[Bhagi rathi]

On Thu, Aug 7, 2008 at 1:49 AM, Dave Chinner <david@fromorbit.com> wrote:

> On Wed, Aug 06, 2008 at 10:42:15PM +0530, Bhagi rathi wrote:
> > I couldn't get a chance to read the diff's completely. If I click on
> > Lachlan's url for diff's, I couldn't access them. It looks to me that
> > the issue is not just with trace buffers. It can extend to xfs_iformat
> > as well. The same dead-lock can spring via
> >
> > xfs_iread -> xfs_iformat -> xfs_iformat_extents -> xfs_iext_add ->
> > xfs_iext_inline_to_direct -> which can do kmem_alloc with
> > KM_SLEEP flag.
>
> Fixed already:
>
>
> http://oss.sgi.com/cgi-bin/gitweb.cgi?p=xfs/xfs-2.6.git;a=commit;h=8c6266658cb76e282c14cb92f8ba5a1c674f4928
>

 Thanks Dave. However, My concern is just not one allocation. We
 need to clean all allocations that can re-enter to file-system.
 I see that this issue exists in attributes format for i_afp allocations.
 It may exist with local format of data and attributes too.

 xfs_iread->xfs_iformat->xfs_iformat_local.

 Are we safe that we fixed all these real problems by looking
 into possible allocations that will enter into file-system? The
 problem with trace buffers is telling us to clean this code path.

 By the way, I browse the source code from lxr.linux.no.
 If I have to browse the latest xfs source code with linux
 kernel that is used at SGI, how can I do that?

 Cheers,
 Bhagi.

>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
>


[[HTML alternate version deleted]]

Re: TAKE 981498 - Use KM_NOFS for debug trace buffers

[Bhagi rathi]

On Thu, Aug 7, 2008 at 2:33 AM, Dave Chinner <david@fromorbit.com> wrote:

> On Thu, Aug 07, 2008 at 06:27:46AM +1000, Dave Chinner wrote:
> > On Thu, Aug 07, 2008 at 06:19:57AM +1000, Dave Chinner wrote:
> > > On Wed, Aug 06, 2008 at 10:42:15PM +0530, Bhagi rathi wrote:
> > > > I couldn't get a chance to read the diff's completely. If I click on
> > > > Lachlan's url for diff's, I couldn't access them. It looks to me that
> > > > the issue is not just with trace buffers. It can extend to
> xfs_iformat
> > > > as well. The same dead-lock can spring via
> > > >
> > > > xfs_iread -> xfs_iformat -> xfs_iformat_extents -> xfs_iext_add ->
> > > > xfs_iext_inline_to_direct -> which can do kmem_alloc with
> > > > KM_SLEEP flag.
> > >
> > > Fixed already:
> > >
> > >
> >
> > Hmmm. where did that url go? Try again:
>
> Ok, something is stripping URLs out of emails. I just sent that URL
> to myself and it wasn't stripped so it's not my mail infrastructure
> that is doing it.
>
> Did someone "upgrade" the spam filters on oss.sgi.com or the
> barracuda overnight?
>
> The link - minus the "http://" bit is:
>
>
> oss.sgi.com/cgi-bin/gitweb.cgi?p=xfs/xfs-2.6.git;a=commit;h=8c6266658cb76e282c14cb92f8ba5a1c674f4928
>

 The above just worked fine for me. Lachlan's  URL still have the same
issue.

 Cheers,
 Bhagi.

>
> <http://oss.sgi.com/cgi-bin/gitweb.cgi?p=xfs/xfs-2.6.git;a=commit;h=8c6266658cb76e282c14cb92f8ba5a1c674f4928>
>
> let's see if that gets stripped....
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
>


[[HTML alternate version deleted]]

Re: TAKE 981498 - Use KM_NOFS for debug trace buffers

[Christoph Hellwig]

Sorry for the later review, but the xfs_super.c changes don't make
sense.  These allocations are done during module_init time and thus
it'ss fien to call back into any fs to reclaim pages for it.
Fortunately the affect is harmless, but I'd suggest reverting that part
of the change.

Re: TAKE 981498 - Use KM_NOFS for debug trace buffers

[Bhagi rathi]

On Thu, Aug 7, 2008 at 11:13 PM, Bhagi rathi <jahnu77@gmail.com> wrote:

>
>
> On Thu, Aug 7, 2008 at 1:49 AM, Dave Chinner <david@fromorbit.com> wrote:
>
>> On Wed, Aug 06, 2008 at 10:42:15PM +0530, Bhagi rathi wrote:
>> > I couldn't get a chance to read the diff's completely. If I click on
>> > Lachlan's url for diff's, I couldn't access them. It looks to me that
>> > the issue is not just with trace buffers. It can extend to xfs_iformat
>> > as well. The same dead-lock can spring via
>> >
>> > xfs_iread -> xfs_iformat -> xfs_iformat_extents -> xfs_iext_add ->
>> > xfs_iext_inline_to_direct -> which can do kmem_alloc with
>> > KM_SLEEP flag.
>>
>> Fixed already:
>>
>>
>> http://oss.sgi.com/cgi-bin/gitweb.cgi?p=xfs/xfs-2.6.git;a=commit;h=8c6266658cb76e282c14cb92f8ba5a1c674f4928
>>
>
>  Thanks Dave. However, My concern is just not one allocation. We
>  need to clean all allocations that can re-enter to file-system.
>  I see that this issue exists in attributes format for i_afp allocations.
>  It may exist with local format of data and attributes too.
>
>  xfs_iread->xfs_iformat->xfs_iformat_local.
>

  Is the above issue fixed already? I see this existing in latest linux
  kernel.


>
>  Are we safe that we fixed all these real problems by looking
>  into possible allocations that will enter into file-system? The
>  problem with trace buffers is telling us to clean this code path.
>
>  By the way, I browse the source code from lxr.linux.no.
>  If I have to browse the latest xfs source code with linux
>  kernel that is used at SGI, how can I do that?
>

 Any pointers on this? I wish to setup my own xfs development enviroment
 and details on this will be helpful.

Cheers,
Bhagi.

>
>
>  Cheers,
>  Bhagi.
>
>>
>> Cheers,
>>
>> Dave.
>> --
>> Dave Chinner
>> david@fromorbit.com
>>
>
>


[[HTML alternate version deleted]]