Linux ocfs2 filesystem development
 help / color / mirror / Atom feed
From: Mark Fasheh <mfasheh@suse.de>
To: ocfs2-devel@oss.oracle.com
Subject: [Ocfs2-devel] [patch 09/15] ocfs2: add functions to add and remove inode in	orphan dir
Date: Thu, 18 Dec 2014 15:18:59 -0800	[thread overview]
Message-ID: <20141218231859.GV7238@wotan.suse.de> (raw)
In-Reply-To: <548f65e2.GaTIVhOoT5YiOhC8%akpm@linux-foundation.org>

Thanks once again for all of this by the way.

On Mon, Dec 15, 2014 at 02:51:14PM -0800, Andrew Morton wrote:
> From: Weiwei Wang <wangww631@huawei.com>
> Subject: ocfs2: add functions to add and remove inode in orphan dir
> 
> Add functions to add inode to orphan dir and remove inode in orphan dir. 
> Here we do not call ocfs2_prepare_orphan_dir and ocfs2_orphan_add
> directly.  Because append O_DIRECT will add inode to orphan two and may
> result in more than one orphan entry for the same inode.

Just so I understand -  your problem is that ocfs2_prepare_orphan_dir() will
EEXIST when it finds the inode already there.

The solution you have below has a couple problems, stemming from the fact
that it uses the same name for an inode that is orphaned because of
nlink == 0 as that of one that is orphaned because it is undergoing direct
IO.

Having identical names in a directory is always considered an error, even
for system directories like the orphan dir. In my honest opinion we should
not do that here either.

Instead you could use a prefix for those entries - something like "dio-".

If you modify ocfs2_prepare_orphan_dir() and __ocfs2_prepare_orphan_dir()
to pass down an optional prefix which is prepended to the name you don't need a
special dio prepare function. Then we don't need ocfs2_dio_prepare_orphan_dir()
any more.

In fact, the EEXIST check now becomes valid again and you don't need to
do all that extra work in ocfs2_dio_orphan_add() to figure out whether the
name is there because of nlink == 0 or we're doing dio (or both!).

Actually, it really becomes trivial to special case dio in
ocfs2_orphan_add() so I would suggest just adding a boolean argument and
doing the bits for dio there, thus allowing us to drop
ocfs2_dio_orphan_add() too.


I hope this all helps.
	--Mark

--
Mark Fasheh

  reply	other threads:[~2014-12-18 23:18 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-15 22:51 [Ocfs2-devel] [patch 09/15] ocfs2: add functions to add and remove inode in orphan dir akpm at linux-foundation.org
2014-12-18 23:18 ` Mark Fasheh [this message]
2014-12-19  3:37   ` Joseph Qi
2014-12-19 20:33     ` Mark Fasheh
2014-12-22  1:04       ` Joseph Qi
2014-12-22  7:06         ` Mark Fasheh
2015-01-13  8:36           ` Joseph Qi
2015-01-16 22:56             ` Mark Fasheh

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=20141218231859.GV7238@wotan.suse.de \
    --to=mfasheh@suse.de \
    --cc=ocfs2-devel@oss.oracle.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