xfs
[Top] [All Lists]

Re: [PATCH] xfs: I/O completion handlers must use NOFS allocations

To: Alex Elder <aelder@xxxxxxx>
Subject: Re: [PATCH] xfs: I/O completion handlers must use NOFS allocations
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Tue, 3 Nov 2009 09:45:46 -0500
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, Thomas Neumann <tneumann@xxxxxxxxxxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <1AB9A794DBDDF54A8A81BE2296F7BDFE83AE5B@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
References: <20091019040003.GA21115@xxxxxxxxxxxxx> <1AB9A794DBDDF54A8A81BE2296F7BDFE83AE5B@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.19 (2009-01-05)
On Mon, Nov 02, 2009 at 02:34:01PM -0600, Alex Elder wrote:
> This looks good.  It's kind of too bad the GFP_ flag
> argument had to be added, but I can't think of a cleaner
> way than the way you did it.

The slightly better way would be to set the PF_FSTRANS flag for the
workqueue threads, but the workqueue interfaces that allows setting
these flags were removed a while ago.

> I have one minor suggestion on wording used in a comment,
> but the code looks correct to me.  I verified that--as you
> say--the only place we're doing an allocation in an I/O
> completion handler (i.e., a function called via
> io_end->io_work->func) is in xfs_iomap_write_unwritten(),
> so this fix should cover the only case.

> > +            * Note that we opencoding the transaction allocation here
> > +            * to pass KM_NOFS - we can't risk to recurse back into
> 
> 
> How about, "we can't risk recursing back into"

Fine with me.  Do you want me to resend or are you going to fix it up on
fly before applying the patch?

<Prev in Thread] Current Thread [Next in Thread>