linux-kernel - Re: [opensuse] nfs_update_inode: inode X mode changed, Y to Z

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080325221321.GC20257@josefsipek.net>
Date:	Tue, 25 Mar 2008 18:13:21 -0400
From:	"Josef 'Jeff' Sipek" <jeffpc@...efsipek.net>
To:	NeilBrown <neilb@...e.de>
Cc:	"J. Bruce Fields" <bfields@...ldses.org>, xfs@....sgi.com,
	Adam Schrotenboer <adam@...00.com>,
	Jesper Juhl <jesper.juhl@...il.com>,
	Trond Myklebust <trond.myklebust@...app.com>,
	linux-kernel@...r.kernel.org, linux-nfs@...r.kernel.org,
	Thomas Daniel <tdaniel@...00.com>,
	Frederic Revenu <frevenu@...00.com>,
	Jeff Doan <jdoan@...00.com>
Subject: Re: [opensuse] nfs_update_inode: inode X mode changed, Y to Z

On Wed, Mar 26, 2008 at 08:38:22AM +1100, NeilBrown wrote:
...
> However you still need to do something about the generation number.  It
> must be set to something.
 
Right.

> When you allocate an inode that doesn't currently exist on the device,
> you obviously cannot increment the old value and use that.

Makes sense.

> However you can do a lot better than always using 0.

I looked at the code (xfs_ialloc.c:xfs_ialloc_ag_alloc)

 290                 /*
 291                  * Set initial values for the inodes in this buffer.
 292                  */
 293                 xfs_biozero(fbuf, 0, ninodes << args.mp->m_sb.sb_inodelog);
 294                 for (i = 0; i < ninodes; i++) {
 295                         free = XFS_MAKE_IPTR(args.mp, fbuf, i);
 296                         free->di_core.di_magic = cpu_to_be16(XFS_DINODE_MAGIC);
 297                         free->di_core.di_version = version;
 298                         free->di_next_unlinked = cpu_to_be32(NULLAGINO);
 299                         xfs_ialloc_log_di(tp, fbuf, i,
 300                                 XFS_DI_CORE_BITS | XFS_DI_NEXT_UNLINKED);
 301                 }

xfs_biozero(...) turns into a memset(buf, 0, len), and since the loop that
follows doesn't change the generation number, it'll stay 0.

> The simplest would be to generate a 'random' number (get_random_bytes).
> Slightly better would be to generate a random number at boot time
> and use that, incrementing it each time it is used to set the
> generation number for an inode.

I'm not familiar enough with NFS, do you want something that's monotonically
increasing or do you just test for inequality?  If it is inequality, why not
just use something like the jiffies - that should be unique enough.

> Even better would be store store that 'next generation number' in the
> superblock so there would be even less risk of the 'random' generation
> producing repeats.
> This is what ext3 does.  It doesn't dynamically allocate inodes,
> but it doesn't want to pay the cost of reading an old inode from
> storage just to see what the generation number is.  So it has
> a number in the superblock which is incremented on each inode allocation
> and is used as the generation number.
 
Something tells me that the SGI folks might not be all too happy with the
in-sb number...XFS tries to be as parallel as possible, and this would cause
the counter variable to bounce around their NUMA systems.  Perhaps a per-ag
variable would be better, but I remember reading that parallelizing updates
to some inode count variable (I forget which) in the superblock
\cite{dchinner-ols2006} led to a rather big improvement.  It's almost
morning down under, so I guess we'll get their comments on this soon.

Josef 'Jeff' Sipek.

-- 
Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are, by
definition, not smart enough to debug it.
		- Brian W. Kernighan 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/