public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Alex Elder <aelder@sgi.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	Thomas Neumann <tneumann@users.sourceforge.net>,
	xfs@oss.sgi.com
Subject: Re: [PATCH] xfs: I/O completion handlers must use NOFS allocations
Date: Tue, 3 Nov 2009 09:45:46 -0500	[thread overview]
Message-ID: <20091103144545.GA32542@infradead.org> (raw)
In-Reply-To: <1AB9A794DBDDF54A8A81BE2296F7BDFE83AE5B@cf--amer001e--3.americas.sgi.com>

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?

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

      reply	other threads:[~2009-11-03 14:45 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-19  4:00 [PATCH] xfs: I/O completion handlers must use NOFS allocations Christoph Hellwig
2009-10-30  8:55 ` Christoph Hellwig
2009-11-02 20:34 ` Alex Elder
2009-11-03 14:45   ` Christoph Hellwig [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20091103144545.GA32542@infradead.org \
    --to=hch@infradead.org \
    --cc=aelder@sgi.com \
    --cc=tneumann@users.sourceforge.net \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox