linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
To: Neil Brown <neilb-l3A5Bk7waGM@public.gmane.org>
Cc: Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [RFC] Reinstate NFS exportability for JFFS2.
Date: Fri, 01 Aug 2008 01:40:30 +0100	[thread overview]
Message-ID: <1217551230.3719.15.camel@shinybook.infradead.org> (raw)
In-Reply-To: <18578.21997.529551.676627-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>

On Fri, 2008-08-01 at 10:16 +1000, Neil Brown wrote:
> On Thursday July 31, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org wrote:
> > On Fri, 2008-05-02 at 11:38 +1000, Neil Brown wrote:
> > > Why is there a deadlock here?
> 
> I was really hoping you would answer this question.

It's because the nfsd readdirplus code recurses into the file system.
>From the file system's ->readdir() function we call back into nfsd's
encode_entry(), which then calls back into the file system's ->lookup()
function so that it can generate a filehandle.

For any file system which has its own internal locking -- which includes
jffs2, btrfs, xfs, jfs, gfs*, ocfs* and probably others -- that
recursive call back into the file system will deadlock.

In the specific case of JFFS2, we need that internal locking because of
lock ordering constraints with the garbage collection -- we need to take
the allocator lock _before_ the per-inode lock, which means we can't use
the generic inode->i_mutex for that purpose. That's documented in
fs/jffs2/README.Locking. I know fewer details about the other affected
file systems.

> I can see the sense in your approach, but it does still seem a bit
> hackish.  I would like to understand the details of the problem enough
> to be confident that there is no less-hackish solution.

I was thinking that we could perhaps get away with an extension to
readdir() that passes the filehandle to its filldir callback too. I'm
not sure if that's sufficient for NFS4 though.

Perhaps we could add a ->lookup_fh() method which is guaranteed to be
called _only_ from within ->readdir(), so it can have (or _lack_) the
appropriate locking.

For now though, this approach moves the problem into the nfsd code where
it belongs -- the VFS never calls into the file system recursively like
this, in the general case.

-- 
dwmw2

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2008-08-01  0:40 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-01 19:42 [RFC] Reinstate NFS exportability for JFFS2 David Woodhouse
2008-05-01 20:48 ` Christoph Hellwig
2008-05-01 22:44   ` David Woodhouse
2008-05-02  1:38     ` Neil Brown
2008-05-02 11:37       ` David Woodhouse
     [not found]         ` <1209728238.25560.686.camel-ZP4jZrcIevRpWr+L1FloEB2eb7JE58TQ@public.gmane.org>
2008-05-02 14:08           ` J. Bruce Fields
2008-07-31 21:54       ` David Woodhouse
2008-08-01  0:16         ` Neil Brown
     [not found]           ` <18578.21997.529551.676627-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2008-08-01  0:40             ` David Woodhouse [this message]
     [not found]               ` <1217551230.3719.15.camel-Fexsq3y4057IgHVZqg5X0TlWvGAXklZc@public.gmane.org>
2008-08-01  0:52                 ` David Woodhouse
2008-08-01  0:53               ` Chuck Lever
     [not found]                 ` <76bd70e30807311753m2785c6d3kd82edd1fe8b5f8b7-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-08-01  1:00                   ` David Woodhouse
     [not found]                     ` <1217552437.3719.30.camel-Fexsq3y4057IgHVZqg5X0TlWvGAXklZc@public.gmane.org>
2008-08-01  1:31                       ` Chuck Lever
2008-08-01  8:13                         ` David Woodhouse
2008-08-01 13:35                         ` David Woodhouse
     [not found]                           ` <1217597759.3454.356.camel-ZP4jZrcIevRpWr+L1FloEB2eb7JE58TQ@public.gmane.org>
2008-08-01 13:56                             ` David Woodhouse
2008-08-01 16:05                               ` Chuck Lever
2008-08-01 16:19                                 ` David Woodhouse
2008-08-01 17:47                                   ` Chuck Lever
2008-08-02 18:26                                     ` J. Bruce Fields
     [not found]                                       ` <20080802182644.GE30454-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2008-08-02 20:42                                         ` David Woodhouse
2008-08-02 21:33                                           ` J. Bruce Fields
     [not found]                                             ` <20080802213337.GA2833-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2008-08-03  8:39                                               ` David Woodhouse
2008-08-03 11:56                                       ` Neil Brown
2008-08-03 17:15                                         ` Chuck Lever
2008-08-04  1:03                                           ` Neil Brown
2008-08-04 18:41                                             ` J. Bruce Fields
2008-08-04 22:37                                               ` Neil Brown
     [not found]                                             ` <18582.21855.2092.903688-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2008-08-04  6:19                                               ` Chuck Lever
2008-08-05  8:51                                                 ` Dave Chinner
2008-08-05  8:59                                                   ` David Woodhouse
2008-08-05  9:47                                                     ` Dave Chinner
2008-08-05 23:06                                                   ` Neil Brown
2008-08-06  0:08                                                     ` Dave Chinner
2008-08-06 19:56                                                       ` J. Bruce Fields
2008-08-06 20:10                                                         ` David Woodhouse
     [not found]                                                           ` <1218053443.5111.148.camel-ZP4jZrcIevRpWr+L1FloEB2eb7JE58TQ@public.gmane.org>
2008-08-09 16:47                                                             ` David Woodhouse
2008-08-09 19:55                                                               ` David Woodhouse
     [not found]                                                                 ` <1218311710.26926.125.camel-ZP4jZrcIevRpWr+L1FloEB2eb7JE58TQ@public.gmane.org>
2008-08-09 20:01                                                                   ` [PATCH 1/4] Factor out nfsd_do_readdir() into its own function David Woodhouse
     [not found]                                                                     ` <1218312114.5063.5.camel-ZP4jZrcIevRpWr+L1FloEB2eb7JE58TQ@public.gmane.org>
2008-08-09 20:07                                                                       ` Christoph Hellwig
2008-08-09 20:02                                                                   ` [PATCH 2/4] Copy XFS readdir hack into nfsd code David Woodhouse
2008-08-09 20:08                                                                     ` Christoph Hellwig
2008-08-09 20:03                                                                   ` [PATCH 3/4] Remove XFS buffered readdir hack David Woodhouse
     [not found]                                                                     ` <1218312191.5063.8.camel-ZP4jZrcIevRpWr+L1FloEB2eb7JE58TQ@public.gmane.org>
2008-08-09 20:09                                                                       ` Christoph Hellwig
2008-08-09 20:03                                                                   ` [PATCH 4/4] Reinstate NFS exportability David Woodhouse
     [not found]                                                                     ` <1218312213.5063.9.camel-ZP4jZrcIevRpWr+L1FloEB2eb7JE58TQ@public.gmane.org>
2008-08-09 20:10                                                                       ` Christoph Hellwig
2008-08-17 18:22                                               ` [RFC] Reinstate NFS exportability for JFFS2 Andreas Dilger
2008-08-01  2:14               ` Neil Brown
     [not found]                 ` <18578.29049.38904.746701-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2008-08-01  8:50                   ` David Woodhouse
2008-08-01 10:03                   ` Al Viro
2008-08-01 23:11                     ` Neil Brown
2008-07-31 21:54       ` [PATCH 1/4] Factor out nfsd_do_readdir() into its own function David Woodhouse
2008-07-31 21:54       ` [PATCH 2/4] Copy XFS readdir hack into nfsd code, introduce FS_NO_LOOKUP_IN_READDIR flag David Woodhouse
2008-07-31 21:55       ` [PATCH 3/4] Switch XFS to using FS_NO_LOOKUP_IN_READDIR, remove local readdir hack David Woodhouse
     [not found]       ` <18458.28833.539314.455215-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2008-07-31 21:55         ` [PATCH 4/4] [JFFS2] Reinstate NFS exportability David Woodhouse

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=1217551230.3719.15.camel@shinybook.infradead.org \
    --to=dwmw2-wegcikhe2lqwvfeawa7xhq@public.gmane.org \
    --cc=hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org \
    --cc=linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=neilb-l3A5Bk7waGM@public.gmane.org \
    /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;
as well as URLs for NNTP newsgroup(s).