[Ocfs2-devel] [patch 09/15] ocfs2: add functions to add and remove inode in orphan dir

[Mark Fasheh]

On Tue, Jan 13, 2015 at 04:36:14PM +0800, Joseph Qi wrote:
> Hi Mark,
> 
> On 2014/12/22 15:06, Mark Fasheh wrote:
> > On Mon, Dec 22, 2014 at 09:04:25AM +0800, Joseph Qi wrote:
> >>>> So after this change, once unlink and dio happen concurrently, there
> >>>> may be two kinds of results:
> >>>> 1. On the same node, only one entry will be added with flag
> >>>> OCFS2_ORPHANED_FL, and flag OCFS2_DIO_ORPHANED_FL will be set and clear
> >>>> during dio.
> >>>> 2. On different nodes, two entries will be added with each flag
> >>>> OCFS2_ORPHANED_FL or OCFS2_DIO_ORPHANED_FL.
> >>>>
> >>>> Since entry name is stringified from blkno and the length is fixed
> >>>> OCFS2_ORPHAN_NAMELEN, so adding a prefix as you suggested may affect
> >>>> too much.
> >>>
> >>> No, adding the prefix is much more preferable to intentionally corrupting
> >>> the conents of a directory. That OCFS2_ORPHAN_NAMELEN is a #define does not
> >>> matter, you are free to make it whatever reasonable value fits the name. The
> >>> directory code naturally handles this, whtether the length is
> >>> OCFS2_ORPHAN_NAMELEN or OCFS2_ORPHAN_NAMELEN + PREFIX_NAMELEN.
> >>>
> >>> Also if you were to run fsck against the file system it will (should) complain
> >>> about having duplciate entries in the orphan dir.
> >>>
> >>> Aside from the corruption issue, there are usability issues too. Someone
> >>> running "ls //orphan_dir:0000" will not be able to tell whether a name is
> >>> there for dio or nlink==0. If you prefix it, then debugging becomes much
> >>> easier.
> >>>
> >> Okay, I will try to take your advice and make a change.
> > 
> > Great, thank you.
> > 
> > 
> >>> Btw, did you see my note about a readonly superblock flag for this? We're
> >>> going to need to handle backward compatibility too.
> >> Yes. At my first glance, it may need a lot change to support feature ro compat
> >> at my first glance. I need to investigate on it before do the actual change.
> >> Thanks.
> > 
> > Kernel should be straight forward - if the flag isn't there just use the
> > existing fallback. Otherwise we can use the new direct_IO code.
> > 
> >>From memory, in ocfs2-tools you'll have to update mkfs to write the flag,
> > tunefs to turn it on/off for an existing filesystem and fsck to truncate the
> > direct-io orphans it finds.
> > 	--Mark
> > 
> For the comments about setting it to a ro compat feature, I still have
> one more questions.
> If I check this feature flag in ocfs2_direct_IO, that means the existing
> formatted ocfs2 volume won't take this feature except we explicitly turn
> it on using tunefs.

Correct.


> But I don't think there is any problem if feature is default on.
> Do you mean we have to treat this feature as optional and let user
> choose?

Yes it should be optional, though we can always have mkfs.ocfs2 turn it on by default.

If the user uses this, and the node crashes with inodes in the orphan dir
they might be deleted by another node with an ealier version of the fs code.
We can not allow that and therefore have to guard with an incompat bit.
	--Mark

--
Mark Fasheh