Re: [RFC] freeing unlinked file indefinitely delayed
Hey Al, On Mon, Jul 13, 2015 at 08:56:46PM +0100, Al Viro wrote: > On Mon, Jul 13, 2015 at 01:17:51PM -0500, Ben Myers wrote: > > > For one thing, this patch does *not* check for i_nlink at all. > > > > I agree that no checking of i_nlink has the advantage of brevity. > > Anyone who is using dentry.d_fsdata with an open_by_handle workload (if > > there are any) will be affected. > > Translate, please. What does d_fsdata have to anything above? If my understanding of your patch is correct, any disconnected dentries will be killed immediately at dput instead of being put on the dentry lru. I have some code that I think may rely on the old behavior with regard to DCACHE_DISCONNECTED. That is, the disconnected dentry sits on the lru and there are lots of open_by_handle calls and nothing to keep a ref on the inode between open_by_handle/close except that dentry's ref. Maybe there are better ways to do that and I just need to look for a workaround. I wonder if there are others who will be affected by this change in behavior. In particular, if there are filesystems that need to keep internal state in d_fsdata across open_by_handle calls. They won't be able to use d_fsdata very well anymore for this workload. > > > For another, there's no such thing as 'filesystems internal lock' for > > > i_nlink protection - that's handled by i_mutex... And what does > > > iget() have to do with any of that? > > > > i_mutex is good enough only for local filesystems. > > Network/clustered/distributed filesystems need to take an internal lock > > to provide exclusion for this .unlink with a .link on another host. > > That's where I'm coming from with iget(). > > > > Maybe plumbing i_op.unlink with another argument to return i_nlink is > > something to consider? A helper for the few filesystems that need to do > > this might be good enough in the near term. > > > > a) iget() had been gone since way back > b) it never had been called by VFS - it's a filesystem's responsibility > c) again, what the hell does iget() or its replacements have to do with > dentry eviction? It does *NOT* affect dentry refcount. Never had. I misspoke with regard to iget. It is the locking for i_nlink that I am concerned about. > d) checks for _inode_ retention in icache are done by filesystem code, which > is certainly free to use its locks. Incidentally, for normal filesystems > no locks are needed at all - everything that changes ->i_nlink is holding > a referfence to in-core inode, so in a situation when its refcount is zero > and ->i_lock is held (providing an exclusion with icache lookups), ->i_nlink > is guaranteed to be stable. Thanks for that explanation, that's what I needed. For filesystems which have a distributed lock manager involved on multiple hosts, i_nlink is not stable on an individual host even with i_lock held. You would also need to hold whatever finer-grained lock the filesystem is protecting i_nlink with. So the VFS _can't_ know when i_nlink has gone to zero, only the filesystem can. That's what I was trying to get at. > e) why would VFS possibly want to know if there are links remaining after > successful ->unlink()? The VFS wouldn't. I guess the argument goes that it would be nice to keep the old behavior and still cache disconnected dentries, and leave it to the filesystems which require it to destroy the anonymous dentries in ->unlink when they they know that i_nlink has gone to zero, having taken whatever internal locks they need to use to adequately protect i_nlink from other hosts. Thats where a helper to prune disconnected dentries would come in handy. > I'm sorry, but you are not making any sense... I'm sorry for the confusion, it took me a little awhile to digest this. The change in caching behavior could affect more than nfs. Any filesystem which expects disconnected dentries to remain cached is affected. Thanks, Ben -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] freeing unlinked file indefinitely delayed
Hey Al, On Sun, Jul 12, 2015 at 04:00:35PM +0100, Al Viro wrote: > On Wed, Jul 08, 2015 at 10:41:43AM -0500, Ben Myers wrote: > > > The bug rings a bell for me so I will stick my neck out instead of > > lurking. Don't you need to sample that link count under the filesystems > > internal lock in order to avoid an unlink/iget race? I suggest creating > > a helper to prune disconnected dentries which a filesystem could call in > > .unlink. That would avoid the risk of unintended side effects with the > > d_alloc/d_free/icache approach and have provable link count correctness. > > For one thing, this patch does *not* check for i_nlink at all. I agree that no checking of i_nlink has the advantage of brevity. Anyone who is using dentry.d_fsdata with an open_by_handle workload (if there are any) will be affected. > For another, there's no such thing as 'filesystems internal lock' for > i_nlink protection - that's handled by i_mutex... And what does > iget() have to do with any of that? i_mutex is good enough only for local filesystems. Network/clustered/distributed filesystems need to take an internal lock to provide exclusion for this .unlink with a .link on another host. That's where I'm coming from with iget(). Maybe plumbing i_op.unlink with another argument to return i_nlink is something to consider? A helper for the few filesystems that need to do this might be good enough in the near term. Thanks, Ben -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] freeing unlinked file indefinitely delayed
Hey Al, On Sun, Jul 12, 2015 at 04:00:35PM +0100, Al Viro wrote: On Wed, Jul 08, 2015 at 10:41:43AM -0500, Ben Myers wrote: The bug rings a bell for me so I will stick my neck out instead of lurking. Don't you need to sample that link count under the filesystems internal lock in order to avoid an unlink/iget race? I suggest creating a helper to prune disconnected dentries which a filesystem could call in .unlink. That would avoid the risk of unintended side effects with the d_alloc/d_free/icache approach and have provable link count correctness. For one thing, this patch does *not* check for i_nlink at all. I agree that no checking of i_nlink has the advantage of brevity. Anyone who is using dentry.d_fsdata with an open_by_handle workload (if there are any) will be affected. For another, there's no such thing as 'filesystems internal lock' for i_nlink protection - that's handled by i_mutex... And what does iget() have to do with any of that? i_mutex is good enough only for local filesystems. Network/clustered/distributed filesystems need to take an internal lock to provide exclusion for this .unlink with a .link on another host. That's where I'm coming from with iget(). Maybe plumbing i_op.unlink with another argument to return i_nlink is something to consider? A helper for the few filesystems that need to do this might be good enough in the near term. Thanks, Ben -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] freeing unlinked file indefinitely delayed
Hey Al, On Mon, Jul 13, 2015 at 08:56:46PM +0100, Al Viro wrote: On Mon, Jul 13, 2015 at 01:17:51PM -0500, Ben Myers wrote: For one thing, this patch does *not* check for i_nlink at all. I agree that no checking of i_nlink has the advantage of brevity. Anyone who is using dentry.d_fsdata with an open_by_handle workload (if there are any) will be affected. Translate, please. What does d_fsdata have to anything above? If my understanding of your patch is correct, any disconnected dentries will be killed immediately at dput instead of being put on the dentry lru. I have some code that I think may rely on the old behavior with regard to DCACHE_DISCONNECTED. That is, the disconnected dentry sits on the lru and there are lots of open_by_handle calls and nothing to keep a ref on the inode between open_by_handle/close except that dentry's ref. Maybe there are better ways to do that and I just need to look for a workaround. I wonder if there are others who will be affected by this change in behavior. In particular, if there are filesystems that need to keep internal state in d_fsdata across open_by_handle calls. They won't be able to use d_fsdata very well anymore for this workload. For another, there's no such thing as 'filesystems internal lock' for i_nlink protection - that's handled by i_mutex... And what does iget() have to do with any of that? i_mutex is good enough only for local filesystems. Network/clustered/distributed filesystems need to take an internal lock to provide exclusion for this .unlink with a .link on another host. That's where I'm coming from with iget(). Maybe plumbing i_op.unlink with another argument to return i_nlink is something to consider? A helper for the few filesystems that need to do this might be good enough in the near term. a) iget() had been gone since way back b) it never had been called by VFS - it's a filesystem's responsibility c) again, what the hell does iget() or its replacements have to do with dentry eviction? It does *NOT* affect dentry refcount. Never had. I misspoke with regard to iget. It is the locking for i_nlink that I am concerned about. d) checks for _inode_ retention in icache are done by filesystem code, which is certainly free to use its locks. Incidentally, for normal filesystems no locks are needed at all - everything that changes -i_nlink is holding a referfence to in-core inode, so in a situation when its refcount is zero and -i_lock is held (providing an exclusion with icache lookups), -i_nlink is guaranteed to be stable. Thanks for that explanation, that's what I needed. For filesystems which have a distributed lock manager involved on multiple hosts, i_nlink is not stable on an individual host even with i_lock held. You would also need to hold whatever finer-grained lock the filesystem is protecting i_nlink with. So the VFS _can't_ know when i_nlink has gone to zero, only the filesystem can. That's what I was trying to get at. e) why would VFS possibly want to know if there are links remaining after successful -unlink()? The VFS wouldn't. I guess the argument goes that it would be nice to keep the old behavior and still cache disconnected dentries, and leave it to the filesystems which require it to destroy the anonymous dentries in -unlink when they they know that i_nlink has gone to zero, having taken whatever internal locks they need to use to adequately protect i_nlink from other hosts. Thats where a helper to prune disconnected dentries would come in handy. I'm sorry, but you are not making any sense... I'm sorry for the confusion, it took me a little awhile to digest this. The change in caching behavior could affect more than nfs. Any filesystem which expects disconnected dentries to remain cached is affected. Thanks, Ben -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] freeing unliked file indefinitely delayed
Hey Al,
On Wed, Jul 08, 2015 at 02:42:38AM +0100, Al Viro wrote:
> Normally opening a file, unlinking it and then closing will have
> the inode freed upon close() (provided that it's not otherwise busy and
> has no remaining links, of course). However, there's one case where that
> does *not* happen. Namely, if you open it by fhandle with cold dcache,
> then unlink() and close().
>
> In normal case you get d_delete() in unlink(2) notice that dentry
> is busy and unhash it; on the final dput() it will be forcibly evicted from
> dcache, triggering iput() and inode removal. In this case, though, we end
> up with *two* dentries - disconnected (created by open-by-fhandle) and
> regular one (used by unlink()). The latter will have its reference to inode
> dropped just fine, but the former will not - it's considered hashed (it
> is on the ->s_anon list), so it will stay around until the memory pressure
> will finally do it in. As the result, we have the final iput() delayed
> indefinitely. It's trivial to reproduce -
>
> #define _GNU_SOURCE
> #include
> #include
> #include
>
> void flush_dcache(void)
> {
> system("mount -o remount,rw /");
> }
>
> static char buf[20 * 1024 * 1024];
>
> main()
> {
> int fd;
> union {
> struct file_handle f;
> char buf[MAX_HANDLE_SZ];
> } x;
> int m;
>
> x.f.handle_bytes = sizeof(x);
> chdir("/root");
> mkdir("foo", 0700);
> fd = open("foo/bar", O_CREAT | O_RDWR, 0600);
> close(fd);
> name_to_handle_at(AT_FDCWD, "foo/bar", , , 0);
> flush_dcache();
> fd = open_by_handle_at(AT_FDCWD, , O_RDWR);
> unlink("foo/bar");
> write(fd, buf, sizeof(buf));
> system("df ."); /* 20Mb eaten */
> close(fd);
> system("df ."); /* should've freed those 20Mb */
> flush_dcache();
> system("df ."); /* should be the same as #2 */
> }
>
> will spit out something like
> Filesystem 1K-blocks Used Available Use% Mounted on
> /dev/root 322023 303843 1131 100% /
> Filesystem 1K-blocks Used Available Use% Mounted on
> /dev/root 322023 303843 1131 100% /
> Filesystem 1K-blocks Used Available Use% Mounted on
> /dev/root 322023 283282 21692 93% /
> - inode gets freed only when dentry is finally evicted (here we trigger
> than by remount; normally it would've happened in response to memory
> pressure hell knows when).
>
> IMO it's a bug. Between the close() and final flush_dcache() the file has
> no surviving links, is *not* busy, won't show up in fuser/lsof/whatnot
> output, and yet it's still not freed. I'm not saying that it's realistically
> exploitable (albeit with nfsd it might be), but it's a very unpleasant
> self-LART.
>
> FWIW, my prefered fix would be simply to have the final dput() treat
> disconnected dentries as "kill on sight"; checking for i_nlink of the
> inode, as Bruce suggested several years ago, will *not* work, simply
> because having another link to that file and unlinking it after close
> will reproduce the situation - disconnected dentry sticks around in
> dcache past its final dput() and past the last unlink() of our file.
> Theoretically it might cause an overhead for nfsd (no_subtree_check v3
> export might see more d_alloc()/d_free(); icache retention will still
> prevent constant rereading the inode from disk). _IF_ that proves to
> be noticable, we might need to come up with something more elaborate
> (e.g. have unlink() and rename() kick disconnected aliases out if the link
> count has reached 0), but it's more complex and needs careful ananlysis
> of correctness - we need to prove that there's no way to miss the link
> count reaching 0.
The bug rings a bell for me so I will stick my neck out instead of
lurking. Don't you need to sample that link count under the filesystems
internal lock in order to avoid an unlink/iget race? I suggest creating
a helper to prune disconnected dentries which a filesystem could call in
.unlink. That would avoid the risk of unintended side effects with the
d_alloc/d_free/icache approach and have provable link count correctness.
Thanks,
Ben
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] freeing unliked file indefinitely delayed
Hey Al,
On Wed, Jul 08, 2015 at 02:42:38AM +0100, Al Viro wrote:
Normally opening a file, unlinking it and then closing will have
the inode freed upon close() (provided that it's not otherwise busy and
has no remaining links, of course). However, there's one case where that
does *not* happen. Namely, if you open it by fhandle with cold dcache,
then unlink() and close().
In normal case you get d_delete() in unlink(2) notice that dentry
is busy and unhash it; on the final dput() it will be forcibly evicted from
dcache, triggering iput() and inode removal. In this case, though, we end
up with *two* dentries - disconnected (created by open-by-fhandle) and
regular one (used by unlink()). The latter will have its reference to inode
dropped just fine, but the former will not - it's considered hashed (it
is on the -s_anon list), so it will stay around until the memory pressure
will finally do it in. As the result, we have the final iput() delayed
indefinitely. It's trivial to reproduce -
#define _GNU_SOURCE
#include sys/types.h
#include sys/stat.h
#include fcntl.h
void flush_dcache(void)
{
system(mount -o remount,rw /);
}
static char buf[20 * 1024 * 1024];
main()
{
int fd;
union {
struct file_handle f;
char buf[MAX_HANDLE_SZ];
} x;
int m;
x.f.handle_bytes = sizeof(x);
chdir(/root);
mkdir(foo, 0700);
fd = open(foo/bar, O_CREAT | O_RDWR, 0600);
close(fd);
name_to_handle_at(AT_FDCWD, foo/bar, x.f, m, 0);
flush_dcache();
fd = open_by_handle_at(AT_FDCWD, x.f, O_RDWR);
unlink(foo/bar);
write(fd, buf, sizeof(buf));
system(df .); /* 20Mb eaten */
close(fd);
system(df .); /* should've freed those 20Mb */
flush_dcache();
system(df .); /* should be the same as #2 */
}
will spit out something like
Filesystem 1K-blocks Used Available Use% Mounted on
/dev/root 322023 303843 1131 100% /
Filesystem 1K-blocks Used Available Use% Mounted on
/dev/root 322023 303843 1131 100% /
Filesystem 1K-blocks Used Available Use% Mounted on
/dev/root 322023 283282 21692 93% /
- inode gets freed only when dentry is finally evicted (here we trigger
than by remount; normally it would've happened in response to memory
pressure hell knows when).
IMO it's a bug. Between the close() and final flush_dcache() the file has
no surviving links, is *not* busy, won't show up in fuser/lsof/whatnot
output, and yet it's still not freed. I'm not saying that it's realistically
exploitable (albeit with nfsd it might be), but it's a very unpleasant
self-LART.
FWIW, my prefered fix would be simply to have the final dput() treat
disconnected dentries as kill on sight; checking for i_nlink of the
inode, as Bruce suggested several years ago, will *not* work, simply
because having another link to that file and unlinking it after close
will reproduce the situation - disconnected dentry sticks around in
dcache past its final dput() and past the last unlink() of our file.
Theoretically it might cause an overhead for nfsd (no_subtree_check v3
export might see more d_alloc()/d_free(); icache retention will still
prevent constant rereading the inode from disk). _IF_ that proves to
be noticable, we might need to come up with something more elaborate
(e.g. have unlink() and rename() kick disconnected aliases out if the link
count has reached 0), but it's more complex and needs careful ananlysis
of correctness - we need to prove that there's no way to miss the link
count reaching 0.
The bug rings a bell for me so I will stick my neck out instead of
lurking. Don't you need to sample that link count under the filesystems
internal lock in order to avoid an unlink/iget race? I suggest creating
a helper to prune disconnected dentries which a filesystem could call in
.unlink. That would avoid the risk of unintended side effects with the
d_alloc/d_free/icache approach and have provable link count correctness.
Thanks,
Ben
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] XFS fix for 3.14-rc2
On Thu, Feb 06, 2014 at 01:34:53PM -0800, Linus Torvalds wrote: > I have no idea why you keep re-sending this. > > It got merged a week ago. See commit f1499382f114. Ok, there it is. Sorry for the noise. Thanks, Ben > On Thu, Feb 6, 2014 at 9:31 AM, Ben Myers wrote: > > Hi Linus, > > > >Please pull this xfs update for 3.14. This is to allow logical sector > > sized > >direct io on advanced format disks. Eric expressed a desire to get this > > fix > >in 3.14 because this bug affects some virtualization packages. > > > > Thanks, > > Ben > > > > The following changes since commit bf3964c188d686424ff7b69a45941851b9f437f0: > > > > Merge branch 'xfs-extent-list-locking-fixes' into for-next (2014-01-09 > > 16:03:18 -0600) > > > > are available in the git repository at: > > > > > > git://oss.sgi.com/xfs/xfs.git tags/xfs-for-linus-v3.14-rc1-2 > > > > for you to fetch changes up to 7c71ee78031c248dca13fc94dea9a4cc217db6cf: > > > > xfs: allow logical-sector sized O_DIRECT (2014-01-24 11:55:42 -0600) > > > > > > xfs: update #2 for v3.14-rc1 > > > > - allow logical sector sized direct io on 'advanced format' 4k/512 disk. > > > > > > Eric Sandeen (3): > > xfs: clean up xfs_buftarg > > xfs: rename xfs_buftarg structure members > > xfs: allow logical-sector sized O_DIRECT > > > > fs/xfs/xfs_buf.c | 14 +- > > fs/xfs/xfs_buf.h | 20 +--- > > fs/xfs/xfs_file.c | 7 +-- > > fs/xfs/xfs_ioctl.c | 2 +- > > 4 files changed, 32 insertions(+), 11 deletions(-) > > ___ > xfs mailing list > x...@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] XFS fix for 3.14-rc2
Hi Linus, Please pull this xfs update for 3.14. This is to allow logical sector sized direct io on advanced format disks. Eric expressed a desire to get this fix in 3.14 because this bug affects some virtualization packages. Thanks, Ben The following changes since commit bf3964c188d686424ff7b69a45941851b9f437f0: Merge branch 'xfs-extent-list-locking-fixes' into for-next (2014-01-09 16:03:18 -0600) are available in the git repository at: git://oss.sgi.com/xfs/xfs.git tags/xfs-for-linus-v3.14-rc1-2 for you to fetch changes up to 7c71ee78031c248dca13fc94dea9a4cc217db6cf: xfs: allow logical-sector sized O_DIRECT (2014-01-24 11:55:42 -0600) xfs: update #2 for v3.14-rc1 - allow logical sector sized direct io on 'advanced format' 4k/512 disk. Eric Sandeen (3): xfs: clean up xfs_buftarg xfs: rename xfs_buftarg structure members xfs: allow logical-sector sized O_DIRECT fs/xfs/xfs_buf.c | 14 +- fs/xfs/xfs_buf.h | 20 +--- fs/xfs/xfs_file.c | 7 +-- fs/xfs/xfs_ioctl.c | 2 +- 4 files changed, 32 insertions(+), 11 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] XFS fix for 3.14-rc2
Hi Linus, Please pull this xfs update for 3.14. This is to allow logical sector sized direct io on advanced format disks. Eric expressed a desire to get this fix in 3.14 because this bug affects some virtualization packages. Thanks, Ben The following changes since commit bf3964c188d686424ff7b69a45941851b9f437f0: Merge branch 'xfs-extent-list-locking-fixes' into for-next (2014-01-09 16:03:18 -0600) are available in the git repository at: git://oss.sgi.com/xfs/xfs.git tags/xfs-for-linus-v3.14-rc1-2 for you to fetch changes up to 7c71ee78031c248dca13fc94dea9a4cc217db6cf: xfs: allow logical-sector sized O_DIRECT (2014-01-24 11:55:42 -0600) xfs: update #2 for v3.14-rc1 - allow logical sector sized direct io on 'advanced format' 4k/512 disk. Eric Sandeen (3): xfs: clean up xfs_buftarg xfs: rename xfs_buftarg structure members xfs: allow logical-sector sized O_DIRECT fs/xfs/xfs_buf.c | 14 +- fs/xfs/xfs_buf.h | 20 +--- fs/xfs/xfs_file.c | 7 +-- fs/xfs/xfs_ioctl.c | 2 +- 4 files changed, 32 insertions(+), 11 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] XFS fix for 3.14-rc2
On Thu, Feb 06, 2014 at 01:34:53PM -0800, Linus Torvalds wrote: I have no idea why you keep re-sending this. It got merged a week ago. See commit f1499382f114. Ok, there it is. Sorry for the noise. Thanks, Ben On Thu, Feb 6, 2014 at 9:31 AM, Ben Myers b...@sgi.com wrote: Hi Linus, Please pull this xfs update for 3.14. This is to allow logical sector sized direct io on advanced format disks. Eric expressed a desire to get this fix in 3.14 because this bug affects some virtualization packages. Thanks, Ben The following changes since commit bf3964c188d686424ff7b69a45941851b9f437f0: Merge branch 'xfs-extent-list-locking-fixes' into for-next (2014-01-09 16:03:18 -0600) are available in the git repository at: git://oss.sgi.com/xfs/xfs.git tags/xfs-for-linus-v3.14-rc1-2 for you to fetch changes up to 7c71ee78031c248dca13fc94dea9a4cc217db6cf: xfs: allow logical-sector sized O_DIRECT (2014-01-24 11:55:42 -0600) xfs: update #2 for v3.14-rc1 - allow logical sector sized direct io on 'advanced format' 4k/512 disk. Eric Sandeen (3): xfs: clean up xfs_buftarg xfs: rename xfs_buftarg structure members xfs: allow logical-sector sized O_DIRECT fs/xfs/xfs_buf.c | 14 +- fs/xfs/xfs_buf.h | 20 +--- fs/xfs/xfs_file.c | 7 +-- fs/xfs/xfs_ioctl.c | 2 +- 4 files changed, 32 insertions(+), 11 deletions(-) ___ xfs mailing list x...@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] XFS update #2 for 3.14-rc1
Hey Linus, On Tue, Jan 28, 2014 at 07:12:09PM -0600, Ben Myers wrote: >Please pull this xfs update for 3.14. This is to allow logical sector > sized >direct io on advanced format disks. Could you pick this up for -rc2? Thanks, Ben > The following changes since commit bf3964c188d686424ff7b69a45941851b9f437f0: > > Merge branch 'xfs-extent-list-locking-fixes' into for-next (2014-01-09 > 16:03:18 -0600) > > are available in the git repository at: > > > git://oss.sgi.com/xfs/xfs.git tags/xfs-for-linus-v3.14-rc1-2 > > for you to fetch changes up to 7c71ee78031c248dca13fc94dea9a4cc217db6cf: > > xfs: allow logical-sector sized O_DIRECT (2014-01-24 11:55:42 -0600) > > > xfs: update #2 for v3.14-rc1 > > - allow logical sector sized direct io on 'advanced format' 4k/512 disk. > > > Eric Sandeen (3): > xfs: clean up xfs_buftarg > xfs: rename xfs_buftarg structure members > xfs: allow logical-sector sized O_DIRECT > > fs/xfs/xfs_buf.c | 14 +- > fs/xfs/xfs_buf.h | 20 +--- > fs/xfs/xfs_file.c | 7 +-- > fs/xfs/xfs_ioctl.c | 2 +- > 4 files changed, 32 insertions(+), 11 deletions(-) > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] XFS update #2 for 3.14-rc1
Hey Linus, On Tue, Jan 28, 2014 at 07:12:09PM -0600, Ben Myers wrote: Please pull this xfs update for 3.14. This is to allow logical sector sized direct io on advanced format disks. Could you pick this up for -rc2? Thanks, Ben The following changes since commit bf3964c188d686424ff7b69a45941851b9f437f0: Merge branch 'xfs-extent-list-locking-fixes' into for-next (2014-01-09 16:03:18 -0600) are available in the git repository at: git://oss.sgi.com/xfs/xfs.git tags/xfs-for-linus-v3.14-rc1-2 for you to fetch changes up to 7c71ee78031c248dca13fc94dea9a4cc217db6cf: xfs: allow logical-sector sized O_DIRECT (2014-01-24 11:55:42 -0600) xfs: update #2 for v3.14-rc1 - allow logical sector sized direct io on 'advanced format' 4k/512 disk. Eric Sandeen (3): xfs: clean up xfs_buftarg xfs: rename xfs_buftarg structure members xfs: allow logical-sector sized O_DIRECT fs/xfs/xfs_buf.c | 14 +- fs/xfs/xfs_buf.h | 20 +--- fs/xfs/xfs_file.c | 7 +-- fs/xfs/xfs_ioctl.c | 2 +- 4 files changed, 32 insertions(+), 11 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] XFS update #2 for 3.14-rc1
Hi Linus, Please pull this xfs update for 3.14. This is to allow logical sector sized direct io on advanced format disks. Thanks, Ben The following changes since commit bf3964c188d686424ff7b69a45941851b9f437f0: Merge branch 'xfs-extent-list-locking-fixes' into for-next (2014-01-09 16:03:18 -0600) are available in the git repository at: git://oss.sgi.com/xfs/xfs.git tags/xfs-for-linus-v3.14-rc1-2 for you to fetch changes up to 7c71ee78031c248dca13fc94dea9a4cc217db6cf: xfs: allow logical-sector sized O_DIRECT (2014-01-24 11:55:42 -0600) xfs: update #2 for v3.14-rc1 - allow logical sector sized direct io on 'advanced format' 4k/512 disk. Eric Sandeen (3): xfs: clean up xfs_buftarg xfs: rename xfs_buftarg structure members xfs: allow logical-sector sized O_DIRECT fs/xfs/xfs_buf.c | 14 +- fs/xfs/xfs_buf.h | 20 +--- fs/xfs/xfs_file.c | 7 +-- fs/xfs/xfs_ioctl.c | 2 +- 4 files changed, 32 insertions(+), 11 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] XFS update #2 for 3.14-rc1
Hi Linus, Please pull this xfs update for 3.14. This is to allow logical sector sized direct io on advanced format disks. Thanks, Ben The following changes since commit bf3964c188d686424ff7b69a45941851b9f437f0: Merge branch 'xfs-extent-list-locking-fixes' into for-next (2014-01-09 16:03:18 -0600) are available in the git repository at: git://oss.sgi.com/xfs/xfs.git tags/xfs-for-linus-v3.14-rc1-2 for you to fetch changes up to 7c71ee78031c248dca13fc94dea9a4cc217db6cf: xfs: allow logical-sector sized O_DIRECT (2014-01-24 11:55:42 -0600) xfs: update #2 for v3.14-rc1 - allow logical sector sized direct io on 'advanced format' 4k/512 disk. Eric Sandeen (3): xfs: clean up xfs_buftarg xfs: rename xfs_buftarg structure members xfs: allow logical-sector sized O_DIRECT fs/xfs/xfs_buf.c | 14 +- fs/xfs/xfs_buf.h | 20 +--- fs/xfs/xfs_file.c | 7 +-- fs/xfs/xfs_ioctl.c | 2 +- 4 files changed, 32 insertions(+), 11 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] XFS updates for 3.14-rc1
Hi Linus, Please pull these XFS updates for 3.14-rc1. This is primarily bug fixes, many of which you already have. New stuff includes a series to decouple the in-memory and on-disk log format, helpers in the area of inode clusters, and i_version handling. We decided to try to use more topic branches this release, so there are some merge commits in there on account of that. I'm afraid I didn't do a good job of putting meaningful comments in the first couple of merges. Sorry about that. I think I have the hang of it now. Thanks, Ben The following changes since commit dc1ccc48159d63eca5089e507c82c7d22ef60839: Linux 3.13-rc2 (2013-11-29 12:57:14 -0800) are available in the git repository at: git://oss.sgi.com/xfs/xfs.git tags/xfs-for-linus-v3.14-rc1 for you to fetch changes up to bf3964c188d686424ff7b69a45941851b9f437f0: Merge branch 'xfs-extent-list-locking-fixes' into for-next (2014-01-09 16:03:18 -0600) xfs: update for v3.14-rc1 For 3.14-rc1 there are fixes in the areas of remote attributes, discard, growfs, memory leaks in recovery, directory v2, quotas, the MAINTAINERS file, allocation alignment, extent list locking, and in xfs_bmapi_allocate. There are cleanups in xfs_setsize_buftarg, removing unused macros, quotas, setattr, and freeing of inode clusters. The in-memory and on-disk log format have been decoupled, a common helper to calculate the number of blocks in an inode cluster has been added, and handling of i_version has been pulled into the filesystems that use it. - cleanup in xfs_setsize_buftarg - removal of remaining unused flags for vop toss/flush/flushinval - fix for memory corruption in xfs_attrlist_by_handle - fix for out-of-date comment in xfs_trans_dqlockedjoin - fix for discard if range length is less than one block - fix for overrun of agfl buffer using growfs on v4 superblock filesystems - pull i_version handling out into the filesystems that use it - don't leak recovery items on error - fix for memory leak in xfs_dir2_node_removename - several cleanups for quotas - fix bad assertion in xfs_qm_vop_create_dqattach - cleanup for xfs_setattr_mode, and add xfs_setattr_time - fix quota assert in xfs_setattr_nonsize - fix an infinite loop when turning off group/project quota before user quota - fix for temporary buffer allocation failure in xfs_dir2_block_to_sf with large directory block sizes - fix Dave's email address in MAINTAINERS - cleanup calculation of freed inode cluster blocks - fix alignment of initial file allocations to match filesystem geometry - decouple in-memory and on-disk log format - introduce a common helper to calculate the number of filesystem blocks in an inode cluster - fixes for extent list locking - fix for off-by-one in xfs_attr3_rmt_verify - fix for missing destroy_work_on_stack in xfs_bmapi_allocate Ben Myers (6): xfs: fix calculation of freed inode cluster blocks Merge branch 'xfs-factor-icluster-macros' into for-next Merge branch 'xfs-for-linus-v3.13-rc5' into for-next xfs: reinstate the ilock in xfs_readdir Merge branch 'xfs-misc' into for-next Merge branch 'xfs-extent-list-locking-fixes' into for-next Christoph Hellwig (26): xfs: remove unused FI_ flags xfs: fix the comment explaining xfs_trans_dqlockedjoin fs: fix iversion handling xfs: tiny xfs_setattr_mode cleanup xfs: add xfs_setattr_time xfs: remove duplicate code in xlog_cil_insert_format_items xfs: refactor xfs_buf_item_format_segment xfs: refactor xfs_inode_item_size xfs: refactor xfs_inode_item_format xfs: introduce xlog_copy_iovec xfs: format log items write directly into the linear CIL buffer xfs: format logged extents directly into the CIL xfs: remove the inode log format from the inode log item xfs: remove the dquot log format from the dquot log item xfs: remove the quotaoff log format from the quotaoff log item xfs: remove xfsbdstrat error xfs: no need to lock the inode in xfs_find_handle xfs: remove xfs_iunlock_map_shared xfs: rename xfs_ilock_map_shared xfs: add xfs_ilock_attr_map_shared xfs: take the ilock around xfs_bmapi_read in xfs_zero_remaining_bytes xfs: use xfs_ilock_data_map_shared in xfs_qm_dqtobp xfs: use xfs_ilock_data_map_shared in xfs_qm_dqiterate xfs: use xfs_ilock_attr_map_shared in xfs_attr_get xfs: use xfs_ilock_attr_map_shared in xfs_attr_list_int xfs: assert that we hold the ilock for extent map access Chuansheng Liu (1): xfs: Calling destroy_work_on_stack() to pair with INIT_WORK_ONSTACK() Dan Carpenter (2): xfs: underflow bug in xfs_attrlist_by_handle() xfs: underflow bug in xfs_attrlist_by_handle() Dave Chinner (7): xfs: growfs overruns AGFL buffer on V4
[GIT PULL] XFS updates for 3.14-rc1
Hi Linus, Please pull these XFS updates for 3.14-rc1. This is primarily bug fixes, many of which you already have. New stuff includes a series to decouple the in-memory and on-disk log format, helpers in the area of inode clusters, and i_version handling. We decided to try to use more topic branches this release, so there are some merge commits in there on account of that. I'm afraid I didn't do a good job of putting meaningful comments in the first couple of merges. Sorry about that. I think I have the hang of it now. Thanks, Ben The following changes since commit dc1ccc48159d63eca5089e507c82c7d22ef60839: Linux 3.13-rc2 (2013-11-29 12:57:14 -0800) are available in the git repository at: git://oss.sgi.com/xfs/xfs.git tags/xfs-for-linus-v3.14-rc1 for you to fetch changes up to bf3964c188d686424ff7b69a45941851b9f437f0: Merge branch 'xfs-extent-list-locking-fixes' into for-next (2014-01-09 16:03:18 -0600) xfs: update for v3.14-rc1 For 3.14-rc1 there are fixes in the areas of remote attributes, discard, growfs, memory leaks in recovery, directory v2, quotas, the MAINTAINERS file, allocation alignment, extent list locking, and in xfs_bmapi_allocate. There are cleanups in xfs_setsize_buftarg, removing unused macros, quotas, setattr, and freeing of inode clusters. The in-memory and on-disk log format have been decoupled, a common helper to calculate the number of blocks in an inode cluster has been added, and handling of i_version has been pulled into the filesystems that use it. - cleanup in xfs_setsize_buftarg - removal of remaining unused flags for vop toss/flush/flushinval - fix for memory corruption in xfs_attrlist_by_handle - fix for out-of-date comment in xfs_trans_dqlockedjoin - fix for discard if range length is less than one block - fix for overrun of agfl buffer using growfs on v4 superblock filesystems - pull i_version handling out into the filesystems that use it - don't leak recovery items on error - fix for memory leak in xfs_dir2_node_removename - several cleanups for quotas - fix bad assertion in xfs_qm_vop_create_dqattach - cleanup for xfs_setattr_mode, and add xfs_setattr_time - fix quota assert in xfs_setattr_nonsize - fix an infinite loop when turning off group/project quota before user quota - fix for temporary buffer allocation failure in xfs_dir2_block_to_sf with large directory block sizes - fix Dave's email address in MAINTAINERS - cleanup calculation of freed inode cluster blocks - fix alignment of initial file allocations to match filesystem geometry - decouple in-memory and on-disk log format - introduce a common helper to calculate the number of filesystem blocks in an inode cluster - fixes for extent list locking - fix for off-by-one in xfs_attr3_rmt_verify - fix for missing destroy_work_on_stack in xfs_bmapi_allocate Ben Myers (6): xfs: fix calculation of freed inode cluster blocks Merge branch 'xfs-factor-icluster-macros' into for-next Merge branch 'xfs-for-linus-v3.13-rc5' into for-next xfs: reinstate the ilock in xfs_readdir Merge branch 'xfs-misc' into for-next Merge branch 'xfs-extent-list-locking-fixes' into for-next Christoph Hellwig (26): xfs: remove unused FI_ flags xfs: fix the comment explaining xfs_trans_dqlockedjoin fs: fix iversion handling xfs: tiny xfs_setattr_mode cleanup xfs: add xfs_setattr_time xfs: remove duplicate code in xlog_cil_insert_format_items xfs: refactor xfs_buf_item_format_segment xfs: refactor xfs_inode_item_size xfs: refactor xfs_inode_item_format xfs: introduce xlog_copy_iovec xfs: format log items write directly into the linear CIL buffer xfs: format logged extents directly into the CIL xfs: remove the inode log format from the inode log item xfs: remove the dquot log format from the dquot log item xfs: remove the quotaoff log format from the quotaoff log item xfs: remove xfsbdstrat error xfs: no need to lock the inode in xfs_find_handle xfs: remove xfs_iunlock_map_shared xfs: rename xfs_ilock_map_shared xfs: add xfs_ilock_attr_map_shared xfs: take the ilock around xfs_bmapi_read in xfs_zero_remaining_bytes xfs: use xfs_ilock_data_map_shared in xfs_qm_dqtobp xfs: use xfs_ilock_data_map_shared in xfs_qm_dqiterate xfs: use xfs_ilock_attr_map_shared in xfs_attr_get xfs: use xfs_ilock_attr_map_shared in xfs_attr_list_int xfs: assert that we hold the ilock for extent map access Chuansheng Liu (1): xfs: Calling destroy_work_on_stack() to pair with INIT_WORK_ONSTACK() Dan Carpenter (2): xfs: underflow bug in xfs_attrlist_by_handle() xfs: underflow bug in xfs_attrlist_by_handle() Dave Chinner (7): xfs: growfs overruns AGFL buffer on V4
[GIT PULL] XFS bugfixes for 3.13-rc8
Hi Linus, Please pull these fixes for XFS. Here we have a bugfix for an off-by-one in the remote attribute verifier that results in a forced shutdown which you can hit with v5 superblock by creating a 64k xattr, and a fix for a missing destroy_work_on_stack() in the allocation worker. It's a bit late, but they are both fairly straightforward. Thanks, Ben The following changes since commit ac8809f9ab01a73de1a47b5a37bd8dcca8712fb3: xfs: abort metadata writeback on permanent errors (2013-12-17 09:40:23 -0600) are available in the git repository at: git://oss.sgi.com/xfs/xfs.git tags/xfs-for-linus-v3.13-rc8 for you to fetch changes up to 1f4a63bf019524c96e79f088cd717b96ef00a249: xfs: Calling destroy_work_on_stack() to pair with INIT_WORK_ONSTACK() (2014-01-10 12:39:38 -0600) xfs: bugfixes for 3.13-rc8 - fix off-by-one in xfs_attr3_rmt_verify - fix missing destroy_work_on_stack() in xfs_bmapi_allocate Chuansheng Liu (1): xfs: Calling destroy_work_on_stack() to pair with INIT_WORK_ONSTACK() Jie Liu (1): xfs: fix off-by-one error in xfs_attr3_rmt_verify fs/xfs/xfs_attr_remote.c | 2 +- fs/xfs/xfs_bmap_util.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] vfs: Fix possible NULL pointer dereference in inode_permission()
Christoph, On Fri, Jan 10, 2014 at 01:31:48AM -0800, Christoph Hellwig wrote: > On Fri, Jan 10, 2014 at 12:06:42AM +, Al Viro wrote: > > Check what XFS is doing ;-/ That's where those call_rcu() have come from. > > Sure, we can separate the simple "just do call_rcu(...->free_inode)" case > > and hit it whenever full ->free_inode is there and ->destroy_inode isn't. > > Not too pretty, but removal of tons of boilerplate might be worth doing > > that anyway. But ->destroy_inode() is still needed for cases where fs > > has its own idea of inode lifetime rules. Again, check what XFS is doing > > in that area... > > Btw, I'd really love to get rid of the XFS ->destroy_inode abuse, it's > been a long time thorn in the flesh. I believe this behavior is related to freeing of an inode cluster. > What's really needed there to make XFS behave more similar to everyone > else is a way for the filesystem to say: "I can't actually free this > inode right now, but I'll come back to you later". This test might read something like: "If my link count has gone to zero, and I am the last inode in my cluster to be freed, and there are other inodes from my cluster incore, I cannot be freed." Should be doable. Maybe there are other reasons. -Ben -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] vfs: Fix possible NULL pointer dereference in inode_permission()
Christoph, On Fri, Jan 10, 2014 at 01:31:48AM -0800, Christoph Hellwig wrote: On Fri, Jan 10, 2014 at 12:06:42AM +, Al Viro wrote: Check what XFS is doing ;-/ That's where those call_rcu() have come from. Sure, we can separate the simple just do call_rcu(...-free_inode) case and hit it whenever full -free_inode is there and -destroy_inode isn't. Not too pretty, but removal of tons of boilerplate might be worth doing that anyway. But -destroy_inode() is still needed for cases where fs has its own idea of inode lifetime rules. Again, check what XFS is doing in that area... Btw, I'd really love to get rid of the XFS -destroy_inode abuse, it's been a long time thorn in the flesh. I believe this behavior is related to freeing of an inode cluster. What's really needed there to make XFS behave more similar to everyone else is a way for the filesystem to say: I can't actually free this inode right now, but I'll come back to you later. This test might read something like: If my link count has gone to zero, and I am the last inode in my cluster to be freed, and there are other inodes from my cluster incore, I cannot be freed. Should be doable. Maybe there are other reasons. -Ben -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] XFS bugfixes for 3.13-rc8
Hi Linus, Please pull these fixes for XFS. Here we have a bugfix for an off-by-one in the remote attribute verifier that results in a forced shutdown which you can hit with v5 superblock by creating a 64k xattr, and a fix for a missing destroy_work_on_stack() in the allocation worker. It's a bit late, but they are both fairly straightforward. Thanks, Ben The following changes since commit ac8809f9ab01a73de1a47b5a37bd8dcca8712fb3: xfs: abort metadata writeback on permanent errors (2013-12-17 09:40:23 -0600) are available in the git repository at: git://oss.sgi.com/xfs/xfs.git tags/xfs-for-linus-v3.13-rc8 for you to fetch changes up to 1f4a63bf019524c96e79f088cd717b96ef00a249: xfs: Calling destroy_work_on_stack() to pair with INIT_WORK_ONSTACK() (2014-01-10 12:39:38 -0600) xfs: bugfixes for 3.13-rc8 - fix off-by-one in xfs_attr3_rmt_verify - fix missing destroy_work_on_stack() in xfs_bmapi_allocate Chuansheng Liu (1): xfs: Calling destroy_work_on_stack() to pair with INIT_WORK_ONSTACK() Jie Liu (1): xfs: fix off-by-one error in xfs_attr3_rmt_verify fs/xfs/xfs_attr_remote.c | 2 +- fs/xfs/xfs_bmap_util.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] xfs: Calling destroy_work_on_stack() to pair with INIT_WORK_ONSTACK()
On Tue, Jan 07, 2014 at 07:21:05PM +0800, Jeff Liu wrote: > Hi Chuansheng, > > On 01/07 2014 16:53 PM, Chuansheng Liu wrote: > > > > In case CONFIG_DEBUG_OBJECTS_WORK is defined, it is needed to > > call destroy_work_on_stack() which frees the debug object to pair > > with INIT_WORK_ONSTACK(). > > > > Signed-off-by: Liu, Chuansheng > > --- > > fs/xfs/xfs_bmap_util.c |1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c > > index 1394106..82e0dab 100644 > > --- a/fs/xfs/xfs_bmap_util.c > > +++ b/fs/xfs/xfs_bmap_util.c > > @@ -287,6 +287,7 @@ xfs_bmapi_allocate( > > INIT_WORK_ONSTACK(>work, xfs_bmapi_allocate_worker); > > queue_work(xfs_alloc_wq, >work); > > wait_for_completion(); > > + destroy_work_on_stack(>work); > > return args->result; > > } > > Thanks for your patch and it work fine for my testing. I missed this in an > old commit: [ 3b876c8f2a xfs: fix debug_object WARN at xfs_alloc_vextent() ] Looks good to me too. Reviewed-by: Ben Myers -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] xfs: Calling destroy_work_on_stack() to pair with INIT_WORK_ONSTACK()
On Tue, Jan 07, 2014 at 07:21:05PM +0800, Jeff Liu wrote: Hi Chuansheng, On 01/07 2014 16:53 PM, Chuansheng Liu wrote: In case CONFIG_DEBUG_OBJECTS_WORK is defined, it is needed to call destroy_work_on_stack() which frees the debug object to pair with INIT_WORK_ONSTACK(). Signed-off-by: Liu, Chuansheng chuansheng@intel.com --- fs/xfs/xfs_bmap_util.c |1 + 1 file changed, 1 insertion(+) diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c index 1394106..82e0dab 100644 --- a/fs/xfs/xfs_bmap_util.c +++ b/fs/xfs/xfs_bmap_util.c @@ -287,6 +287,7 @@ xfs_bmapi_allocate( INIT_WORK_ONSTACK(args-work, xfs_bmapi_allocate_worker); queue_work(xfs_alloc_wq, args-work); wait_for_completion(done); + destroy_work_on_stack(args-work); return args-result; } Thanks for your patch and it work fine for my testing. I missed this in an old commit: [ 3b876c8f2a xfs: fix debug_object WARN at xfs_alloc_vextent() ] Looks good to me too. Reviewed-by: Ben Myers b...@sgi.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] XFS bugfixes for 3.13-rc5
Hi Linus, Please pull these fixes for XFS. This contains fixes for some asserts related to project quotas, a memory leak, a hang when disabling group or project quotas before disabling user quotas, Dave's email address, several fixes for the alignment of file allocation to stripe unit/width geometry, a fix for an assertion with xfs_zero_remaining_bytes, and the behavior of metadata writeback in the face of IO errors. Thanks, Ben The following changes since commit f94c44573e7c22860e2c3dfe349c45f72ba35ad3: xfs: growfs overruns AGFL buffer on V4 filesystems (2013-12-10 10:04:27 -0600) are available in the git repository at: git://oss.sgi.com/xfs/xfs.git tags/xfs-for-linus-v3.13-rc5 for you to fetch changes up to ac8809f9ab01a73de1a47b5a37bd8dcca8712fb3: xfs: abort metadata writeback on permanent errors (2013-12-17 09:40:23 -0600) xfs: bugfixes for 3.13-rc5 - fix memory leak in xfs_dir2_node_removename - fix quota assertion in xfs_setattr_size - fix quota assertions in xfs_qm_vop_create_dqattach - fix for hang when disabling group and project quotas before disabling user quotas - fix Dave Chinner's email address in MAINTAINERS - fix for file allocation alignment - fix for assertion in xfs_buf_stale by removing xfsbdstrat - fix for alignment with swalloc mount option - fix for "retry forever" semantics on IO errors Christoph Hellwig (1): xfs: remove xfsbdstrat error Dave Chinner (3): xfs: align initial file allocations correctly xfs: swalloc doesn't align allocations properly xfs: abort metadata writeback on permanent errors Jie Liu (3): xfs: fix false assertion at xfs_qm_vop_create_dqattach xfs: fix assertion failure at xfs_setattr_nonsize xfs: fix infinite loop by detaching the group/project hints from user dquot Mark Tinguely (1): xfs: fix memory leak in xfs_dir2_node_removename Namjae Jeon (1): MAINTAINERS: fix incorrect mail address of XFS maintainer MAINTAINERS | 2 +- fs/xfs/xfs_bmap.c| 32 ++- fs/xfs/xfs_bmap_util.c | 14 +++-- fs/xfs/xfs_buf.c | 37 +- fs/xfs/xfs_buf.h | 11 --- fs/xfs/xfs_buf_item.c| 21 +++-- fs/xfs/xfs_dir2_node.c | 26 fs/xfs/xfs_iops.c| 3 +- fs/xfs/xfs_log_recover.c | 13 ++-- fs/xfs/xfs_qm.c | 80 fs/xfs/xfs_trans_buf.c | 13 +++- 11 files changed, 168 insertions(+), 84 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] XFS bugfixes for 3.13-rc5
Hi Linus, Please pull these fixes for XFS. This contains fixes for some asserts related to project quotas, a memory leak, a hang when disabling group or project quotas before disabling user quotas, Dave's email address, several fixes for the alignment of file allocation to stripe unit/width geometry, a fix for an assertion with xfs_zero_remaining_bytes, and the behavior of metadata writeback in the face of IO errors. Thanks, Ben The following changes since commit f94c44573e7c22860e2c3dfe349c45f72ba35ad3: xfs: growfs overruns AGFL buffer on V4 filesystems (2013-12-10 10:04:27 -0600) are available in the git repository at: git://oss.sgi.com/xfs/xfs.git tags/xfs-for-linus-v3.13-rc5 for you to fetch changes up to ac8809f9ab01a73de1a47b5a37bd8dcca8712fb3: xfs: abort metadata writeback on permanent errors (2013-12-17 09:40:23 -0600) xfs: bugfixes for 3.13-rc5 - fix memory leak in xfs_dir2_node_removename - fix quota assertion in xfs_setattr_size - fix quota assertions in xfs_qm_vop_create_dqattach - fix for hang when disabling group and project quotas before disabling user quotas - fix Dave Chinner's email address in MAINTAINERS - fix for file allocation alignment - fix for assertion in xfs_buf_stale by removing xfsbdstrat - fix for alignment with swalloc mount option - fix for retry forever semantics on IO errors Christoph Hellwig (1): xfs: remove xfsbdstrat error Dave Chinner (3): xfs: align initial file allocations correctly xfs: swalloc doesn't align allocations properly xfs: abort metadata writeback on permanent errors Jie Liu (3): xfs: fix false assertion at xfs_qm_vop_create_dqattach xfs: fix assertion failure at xfs_setattr_nonsize xfs: fix infinite loop by detaching the group/project hints from user dquot Mark Tinguely (1): xfs: fix memory leak in xfs_dir2_node_removename Namjae Jeon (1): MAINTAINERS: fix incorrect mail address of XFS maintainer MAINTAINERS | 2 +- fs/xfs/xfs_bmap.c| 32 ++- fs/xfs/xfs_bmap_util.c | 14 +++-- fs/xfs/xfs_buf.c | 37 +- fs/xfs/xfs_buf.h | 11 --- fs/xfs/xfs_buf_item.c| 21 +++-- fs/xfs/xfs_dir2_node.c | 26 fs/xfs/xfs_iops.c| 3 +- fs/xfs/xfs_log_recover.c | 13 ++-- fs/xfs/xfs_qm.c | 80 fs/xfs/xfs_trans_buf.c | 13 +++- 11 files changed, 168 insertions(+), 84 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] XFS bugfixes for 3.13-rc4
Hi Linus, Please pull these bugfixes for xfs. There is a security fix for memory corruption in xfs_attrlist_by_handle, a fix for trim, and a fix for buffer overrun in the agfl when growing filesystems with a v4 superblock. Thanks, Ben The following changes since commit dc1ccc48159d63eca5089e507c82c7d22ef60839: Linux 3.13-rc2 (2013-11-29 12:57:14 -0800) are available in the git repository at: git://oss.sgi.com/xfs/xfs.git tags/xfs-for-linus-v3.13-rc4 for you to fetch changes up to f94c44573e7c22860e2c3dfe349c45f72ba35ad3: xfs: growfs overruns AGFL buffer on V4 filesystems (2013-12-10 10:04:27 -0600) xfs: bugfixes for 3.13-rc4 - fix for buffer overrun in agfl with growfs on v4 superblock - return EINVAL if requested discard length is less than a block - fix possible memory corruption in xfs_attrlist_by_handle() Dan Carpenter (1): xfs: underflow bug in xfs_attrlist_by_handle() Dave Chinner (1): xfs: growfs overruns AGFL buffer on V4 filesystems Jie Liu (1): xfs: don't perform discard if the given range length is less than block size fs/xfs/xfs_discard.c | 5 +++-- fs/xfs/xfs_fsops.c | 6 +- fs/xfs/xfs_ioctl.c | 3 ++- fs/xfs/xfs_ioctl32.c | 3 ++- 4 files changed, 12 insertions(+), 5 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: XFS security fix never sent to -stable?
Hi,
On Tue, Dec 10, 2013 at 08:31:23AM -0600, Eric Sandeen wrote:
> On 12/10/13, 7:15 AM, Josh Boyer wrote:
> > On Tue, Dec 10, 2013 at 2:56 AM, Greg KH wrote:
> >> On Tue, Dec 10, 2013 at 10:55:23AM +1100, Dave Chinner wrote:
> >>> [cc xfs list, cc sta...@vger.kernel.org]
> >>>
> >>> On Mon, Dec 09, 2013 at 08:17:09AM -0500, Josh Boyer wrote:
> On Mon, Dec 9, 2013 at 7:15 AM, Luis Henriques
> wrote:
> > On Thu, Dec 05, 2013 at 04:35:50PM -0800, Kees Cook wrote:
> >> Hi,
> >>
> >> It looks like 8c567a7fab6e086a0284eee2db82348521e7120c ("xfs: add
> >> capability check to free eofblocks ioctl") is a security fix that was
> >> never sent to -stable? From what I can see, it was introduced in 3.8
> >> by 8ca149de80478441352a8622ea15fae7de703ced ("xfs: add
> >> XFS_IOC_FREE_EOFBLOCKS ioctl").
> >>
> >> I don't see this in the 3.8.y tree. Should it be added there and newer?
> >
> > Thanks Kees, I'm queuing it for the 3.11 kernel.
>
> There's also this one:
>
> http://thread.gmane.org/gmane.comp.file-systems.xfs.general/57654
>
> It fixes CVE-2013-6382
> >>>
> >>> First I've heard about it there being a CVE for that bug. Since when
> >>> has it been considered best practice to publish CVEs without first
> >>> (or ever) directly contacting the relevant upstream developers?
> >>>
> >>> But, regardless of how broken I think the CVE process is, commit
> >>> 071c529 ("xfs: underflow bug in xfs_attrlist_by_handle()") should be
> >>> picked up by the stable kernels.
> >>
> >> I don't see that commit in Linus's tree, is it not there yet?
> >
> > Not yet. Ben said it's applied but I'm not sure where that is.
>
> xfs git tree:
>
> http://oss.sgi.com/cgi-bin/gitweb.cgi?p=xfs/xfs.git;a=commitdiff;h=071c529eb672648ee8ca3f90944bcbcc730b4c06
I'll send a pull request containing this commit this afternoon.
Thanks,
Ben
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] XFS bugfixes for 3.13-rc4
Hi Linus, Please pull these bugfixes for xfs. There is a security fix for memory corruption in xfs_attrlist_by_handle, a fix for trim, and a fix for buffer overrun in the agfl when growing filesystems with a v4 superblock. Thanks, Ben The following changes since commit dc1ccc48159d63eca5089e507c82c7d22ef60839: Linux 3.13-rc2 (2013-11-29 12:57:14 -0800) are available in the git repository at: git://oss.sgi.com/xfs/xfs.git tags/xfs-for-linus-v3.13-rc4 for you to fetch changes up to f94c44573e7c22860e2c3dfe349c45f72ba35ad3: xfs: growfs overruns AGFL buffer on V4 filesystems (2013-12-10 10:04:27 -0600) xfs: bugfixes for 3.13-rc4 - fix for buffer overrun in agfl with growfs on v4 superblock - return EINVAL if requested discard length is less than a block - fix possible memory corruption in xfs_attrlist_by_handle() Dan Carpenter (1): xfs: underflow bug in xfs_attrlist_by_handle() Dave Chinner (1): xfs: growfs overruns AGFL buffer on V4 filesystems Jie Liu (1): xfs: don't perform discard if the given range length is less than block size fs/xfs/xfs_discard.c | 5 +++-- fs/xfs/xfs_fsops.c | 6 +- fs/xfs/xfs_ioctl.c | 3 ++- fs/xfs/xfs_ioctl32.c | 3 ++- 4 files changed, 12 insertions(+), 5 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: XFS security fix never sent to -stable?
Hi, On Tue, Dec 10, 2013 at 08:31:23AM -0600, Eric Sandeen wrote: On 12/10/13, 7:15 AM, Josh Boyer wrote: On Tue, Dec 10, 2013 at 2:56 AM, Greg KH gre...@linuxfoundation.org wrote: On Tue, Dec 10, 2013 at 10:55:23AM +1100, Dave Chinner wrote: [cc xfs list, cc sta...@vger.kernel.org] On Mon, Dec 09, 2013 at 08:17:09AM -0500, Josh Boyer wrote: On Mon, Dec 9, 2013 at 7:15 AM, Luis Henriques luis.henriq...@canonical.com wrote: On Thu, Dec 05, 2013 at 04:35:50PM -0800, Kees Cook wrote: Hi, It looks like 8c567a7fab6e086a0284eee2db82348521e7120c (xfs: add capability check to free eofblocks ioctl) is a security fix that was never sent to -stable? From what I can see, it was introduced in 3.8 by 8ca149de80478441352a8622ea15fae7de703ced (xfs: add XFS_IOC_FREE_EOFBLOCKS ioctl). I don't see this in the 3.8.y tree. Should it be added there and newer? Thanks Kees, I'm queuing it for the 3.11 kernel. There's also this one: http://thread.gmane.org/gmane.comp.file-systems.xfs.general/57654 It fixes CVE-2013-6382 First I've heard about it there being a CVE for that bug. Since when has it been considered best practice to publish CVEs without first (or ever) directly contacting the relevant upstream developers? But, regardless of how broken I think the CVE process is, commit 071c529 (xfs: underflow bug in xfs_attrlist_by_handle()) should be picked up by the stable kernels. I don't see that commit in Linus's tree, is it not there yet? Not yet. Ben said it's applied but I'm not sure where that is. xfs git tree: http://oss.sgi.com/cgi-bin/gitweb.cgi?p=xfs/xfs.git;a=commitdiff;h=071c529eb672648ee8ca3f90944bcbcc730b4c06 I'll send a pull request containing this commit this afternoon. Thanks, Ben -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] update xfs maintainers
On Thu, Nov 14, 2013 at 05:22:26PM +1100, Dave Chinner wrote: > On Fri, Nov 08, 2013 at 04:03:10PM -0600, Ben Myers wrote: > > From: Ben Myers > > > > xfs: update maintainers > > > > Add Dave as maintainer of XFS. > > > > Signed-off-by: Ben Myers > > --- > > MAINTAINERS |2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > Index: b/MAINTAINERS > > === > > --- a/MAINTAINERS 2013-11-08 15:20:18.935186245 -0600 > > +++ b/MAINTAINERS 2013-11-08 15:22:50.685245977 -0600 > > @@ -9387,8 +9387,8 @@ F:drivers/xen/*swiotlb* > > > > XFS FILESYSTEM > > P: Silicon Graphics Inc > > +M: Dave Chinner > > M: Ben Myers > > -M: Alex Elder > > M: x...@oss.sgi.com > > L: x...@oss.sgi.com > > W: http://oss.sgi.com/projects/xfs > > Reviewed-by: Dave Chinner Applied. ;) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] update xfs maintainers
On Thu, Nov 14, 2013 at 05:22:26PM +1100, Dave Chinner wrote: On Fri, Nov 08, 2013 at 04:03:10PM -0600, Ben Myers wrote: From: Ben Myers b...@sgi.com xfs: update maintainers Add Dave as maintainer of XFS. Signed-off-by: Ben Myers b...@sgi.com --- MAINTAINERS |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: b/MAINTAINERS === --- a/MAINTAINERS 2013-11-08 15:20:18.935186245 -0600 +++ b/MAINTAINERS 2013-11-08 15:22:50.685245977 -0600 @@ -9387,8 +9387,8 @@ F:drivers/xen/*swiotlb* XFS FILESYSTEM P: Silicon Graphics Inc +M: Dave Chinner dchin...@fromorbit.com M: Ben Myers b...@sgi.com -M: Alex Elder el...@kernel.org M: x...@oss.sgi.com L: x...@oss.sgi.com W: http://oss.sgi.com/projects/xfs Reviewed-by: Dave Chinner da...@fromorbit.com Applied. ;) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] XFS update for 3.13-rc1
Hi Linus,
Please pull these XFS updates for 3.13-rc1. It's kind of a random
assortment. There is more rearrangement to make libxfs sync with the
kernel, the differences between v2 and v3 were abstracted into an ops
vector, xfs_inactive was reworked, along with the preallocation and hole
punch codepaths. Plenty of bugfixes, and cleanups too.
Thanks,
Ben
The following changes since commit 272b98c6455f00884f0350f775c5342358ebb73f:
Linux 3.12-rc1 (2013-09-16 16:17:51 -0400)
are available in the git repository at:
git://oss.sgi.com/xfs/xfs.git tags/xfs-for-linus-v3.13-rc1
for you to fetch changes up to 359d992bcd398273637cd9edde10afca953783c4:
xfs: simplify kmem_{zone_}zalloc (2013-11-06 16:31:27 -0600)
xfs: update for v3.13-rc1
For 3.13-rc1 we have an eclectic assortment of bugfixes, cleanups, and
refactoring. Bugfixes that stand out are the fix for the AGF/AGI
deadlock, incore extent list fixes, verifier fixes for v4 superblocks
and growfs, and memory leaks. There are some asserts, warnings, and
strings that were cleaned up. There was further rearrangement of code
to make libxfs and the kernel sync up more easily, differences between
v2 and v3 directory code were abstracted using an ops vector,
xfs_inactive was reworked, and the preallocation/hole punching code was
refactored.
- simplify kmem_zone_zalloc
- add traces for AGF/AGI read ops
- add additional AIL traces
- fix xfs_remove AGF vs AGI deadlock
- fix the extent count of new incore extent page in the indirection array
- don't fail bad secondary superblocks verification on v4 filesystems
due to unzeroed bits after v4 fields
- fix possible NULL dereference in xlog_verify_iclog
- remove redundant assert in xfs_dir2_leafn_split
- prevent stack overflows from page cache allocation
- fix some sparse warnings
- fix directory block format verifier to check the leaf entry count
- abstract the differences in dir2/dir3 via an ops vector
- continue process of reorganization to make libxfs/kernel code merges easier
- refactor the preallocation and hole punching code
- fix for growfs and verifiers
- remove unnecessary scary corruption error when probing non-xfs filesystems
- remove extra newlines from strings passed to printk
- prevent deadlock trying to cover an active log
- rework xfs_inactive()
- add the inode directory type support to XFS_IOC_FSGEOM
- cleanup (remove) usage of is_bad_inode
- fix miscalculation in xfs_iext_realloc_direct which results in oversized
direct extent list
- remove unnecessary count arg to xfs_iomap_write_allocate
- fix memory leak in xlog_recover_add_to_trans
- check superblock instead of block magic to determine if dtype field
is present
- fix lockdep annotation due to project quotas
- fix regression in xfs_node_toosmall which can lead to incorrect directory
btree node collapse
- make log recovery verify filesystem uuid of recovering blocks
- fix XFS_IOC_FREE_EOFBLOCKS definition
- remove invalid assert in xfs_inode_free
- fix for AIL lock regression
Ben Myers (1):
xfs: remove usage of is_bad_inode
Brian Foster (4):
xfs: push down inactive transaction mgmt for remote symlinks
xfs: push down inactive transaction mgmt for truncate
xfs: push down inactive transaction mgmt for ifree
xfs: clean up xfs_inactive() error handling, kill VN_INACTIVE_[NO]CACHE
Christoph Hellwig (5):
xfs: always take the iolock around xfs_setattr_size
xfs: remove the unused XFS_ATTR_NONBLOCK flag
xfs: always hold the iolock when calling xfs_change_file_space
xfs: simplify the fallocate path
xfs: fold xfs_change_file_space into xfs_ioc_space
Dave Chinner (30):
xfs: lock the AIL before removing the buffer item
xfs: asserting lock not held during freeing not valid
xfs: fix XFS_IOC_FREE_EOFBLOCKS definition
xfs: log recovery lsn ordering needs uuid check
xfs: lockdep needs to know about 3 dquot-deep nesting
xfs: dirent dtype presence is dependent on directory magic numbers
xfs: prevent deadlock trying to cover an active log
xfs: create a shared header file for format-related information
xfs: unify directory/attribute format definitions
xfs: split dquot buffer operations out
xfs: remove unused transaction callback variables
xfs: decouple log and transaction headers
xfs: decouple inode and bmap btree header files
xfs: split xfs_rtalloc.c for userspace sanity
xfs: abstract the differences in dir2/dir3 via an ops vector
xfs: vectorise remaining shortform dir2 ops
xfs: vectorise directory data operations
xfs: vectorise directory data operations part 2
xfs: vectorise directory leaf operations
xfs: vectorise DA btree operations
xfs: vectorise encoding/decoding directory headers
xfs: convert directory vector
[GIT PULL] XFS update for 3.13-rc1
Hi Linus,
Please pull these XFS updates for 3.13-rc1. It's kind of a random
assortment. There is more rearrangement to make libxfs sync with the
kernel, the differences between v2 and v3 were abstracted into an ops
vector, xfs_inactive was reworked, along with the preallocation and hole
punch codepaths. Plenty of bugfixes, and cleanups too.
Thanks,
Ben
The following changes since commit 272b98c6455f00884f0350f775c5342358ebb73f:
Linux 3.12-rc1 (2013-09-16 16:17:51 -0400)
are available in the git repository at:
git://oss.sgi.com/xfs/xfs.git tags/xfs-for-linus-v3.13-rc1
for you to fetch changes up to 359d992bcd398273637cd9edde10afca953783c4:
xfs: simplify kmem_{zone_}zalloc (2013-11-06 16:31:27 -0600)
xfs: update for v3.13-rc1
For 3.13-rc1 we have an eclectic assortment of bugfixes, cleanups, and
refactoring. Bugfixes that stand out are the fix for the AGF/AGI
deadlock, incore extent list fixes, verifier fixes for v4 superblocks
and growfs, and memory leaks. There are some asserts, warnings, and
strings that were cleaned up. There was further rearrangement of code
to make libxfs and the kernel sync up more easily, differences between
v2 and v3 directory code were abstracted using an ops vector,
xfs_inactive was reworked, and the preallocation/hole punching code was
refactored.
- simplify kmem_zone_zalloc
- add traces for AGF/AGI read ops
- add additional AIL traces
- fix xfs_remove AGF vs AGI deadlock
- fix the extent count of new incore extent page in the indirection array
- don't fail bad secondary superblocks verification on v4 filesystems
due to unzeroed bits after v4 fields
- fix possible NULL dereference in xlog_verify_iclog
- remove redundant assert in xfs_dir2_leafn_split
- prevent stack overflows from page cache allocation
- fix some sparse warnings
- fix directory block format verifier to check the leaf entry count
- abstract the differences in dir2/dir3 via an ops vector
- continue process of reorganization to make libxfs/kernel code merges easier
- refactor the preallocation and hole punching code
- fix for growfs and verifiers
- remove unnecessary scary corruption error when probing non-xfs filesystems
- remove extra newlines from strings passed to printk
- prevent deadlock trying to cover an active log
- rework xfs_inactive()
- add the inode directory type support to XFS_IOC_FSGEOM
- cleanup (remove) usage of is_bad_inode
- fix miscalculation in xfs_iext_realloc_direct which results in oversized
direct extent list
- remove unnecessary count arg to xfs_iomap_write_allocate
- fix memory leak in xlog_recover_add_to_trans
- check superblock instead of block magic to determine if dtype field
is present
- fix lockdep annotation due to project quotas
- fix regression in xfs_node_toosmall which can lead to incorrect directory
btree node collapse
- make log recovery verify filesystem uuid of recovering blocks
- fix XFS_IOC_FREE_EOFBLOCKS definition
- remove invalid assert in xfs_inode_free
- fix for AIL lock regression
Ben Myers (1):
xfs: remove usage of is_bad_inode
Brian Foster (4):
xfs: push down inactive transaction mgmt for remote symlinks
xfs: push down inactive transaction mgmt for truncate
xfs: push down inactive transaction mgmt for ifree
xfs: clean up xfs_inactive() error handling, kill VN_INACTIVE_[NO]CACHE
Christoph Hellwig (5):
xfs: always take the iolock around xfs_setattr_size
xfs: remove the unused XFS_ATTR_NONBLOCK flag
xfs: always hold the iolock when calling xfs_change_file_space
xfs: simplify the fallocate path
xfs: fold xfs_change_file_space into xfs_ioc_space
Dave Chinner (30):
xfs: lock the AIL before removing the buffer item
xfs: asserting lock not held during freeing not valid
xfs: fix XFS_IOC_FREE_EOFBLOCKS definition
xfs: log recovery lsn ordering needs uuid check
xfs: lockdep needs to know about 3 dquot-deep nesting
xfs: dirent dtype presence is dependent on directory magic numbers
xfs: prevent deadlock trying to cover an active log
xfs: create a shared header file for format-related information
xfs: unify directory/attribute format definitions
xfs: split dquot buffer operations out
xfs: remove unused transaction callback variables
xfs: decouple log and transaction headers
xfs: decouple inode and bmap btree header files
xfs: split xfs_rtalloc.c for userspace sanity
xfs: abstract the differences in dir2/dir3 via an ops vector
xfs: vectorise remaining shortform dir2 ops
xfs: vectorise directory data operations
xfs: vectorise directory data operations part 2
xfs: vectorise directory leaf operations
xfs: vectorise DA btree operations
xfs: vectorise encoding/decoding directory headers
xfs: convert directory vector
Re: XFS leadership and a new co-maintainer candidate
Hey, On Tue, Nov 12, 2013 at 09:32:53AM -0800, Christoph Hellwig wrote: > On Fri, Nov 08, 2013 at 02:46:06PM -0600, Ben Myers wrote: > > That really didn't happen Christoph. It's not in my tree or in a pull > > request. > > I'll take my back room complain back then, but I still think that this > is not a useful way to discuss something like this. Thanks. Tact, Ben, Tact. ;) > > Linus, let me know what you want to do. I do think we're doing a fair job > > over here, and (geez) I'm just trying to add Mark as my backup since Alex > > is too busy. I know the RH people want more control, and that's > > understandable, but they really don't need to replace me to get their code > > in. Ouch. > > I'd really like to see more diversity in XFS maintainers. The SGI focus has > defintively been an issue again and again because it seems when one SGI > person is too busy the others usually are as well. As mentioned before > there's also been historically a way too high turnover, with the associated > transition pains. I think diversity in XFS maintainers is a great idea. How wide of a net are you suggesting we cast? I guess it sort of depends upon what you feel is the purpose of the file. > By making sure we have a broader base for the maintainers, and a more open > infrastructure we'll all win. Agreed. > Note that we already had that sort > of instructure on kernel.org, but gave up on it because many people > perceived the effort to re-gain the kernel.org accounts to high. It is a little difficult to find your way into the web of trust. Not everyone is in a position to make way to a conference, or to meet people in person. And even then it can be intimidating to ask for a signature. > I would also really like to get a clarification on "I know the RH people want > more control, and that's understandable, but they really don't need to > replace me to get their code in". What specific people are you worried about > an what code? What makes "the RH people" less worthy to their code in than > "the SGI" people. I'm convinced we're having this discussion for the right reasons, so let's let that line of discussion die where it is. Regards, Ben -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: XFS leadership and a new co-maintainer candidate
Hey, On Tue, Nov 12, 2013 at 09:32:53AM -0800, Christoph Hellwig wrote: On Fri, Nov 08, 2013 at 02:46:06PM -0600, Ben Myers wrote: That really didn't happen Christoph. It's not in my tree or in a pull request. I'll take my back room complain back then, but I still think that this is not a useful way to discuss something like this. Thanks. Tact, Ben, Tact. ;) Linus, let me know what you want to do. I do think we're doing a fair job over here, and (geez) I'm just trying to add Mark as my backup since Alex is too busy. I know the RH people want more control, and that's understandable, but they really don't need to replace me to get their code in. Ouch. I'd really like to see more diversity in XFS maintainers. The SGI focus has defintively been an issue again and again because it seems when one SGI person is too busy the others usually are as well. As mentioned before there's also been historically a way too high turnover, with the associated transition pains. I think diversity in XFS maintainers is a great idea. How wide of a net are you suggesting we cast? I guess it sort of depends upon what you feel is the purpose of the file. By making sure we have a broader base for the maintainers, and a more open infrastructure we'll all win. Agreed. Note that we already had that sort of instructure on kernel.org, but gave up on it because many people perceived the effort to re-gain the kernel.org accounts to high. It is a little difficult to find your way into the web of trust. Not everyone is in a position to make way to a conference, or to meet people in person. And even then it can be intimidating to ask for a signature. I would also really like to get a clarification on I know the RH people want more control, and that's understandable, but they really don't need to replace me to get their code in. What specific people are you worried about an what code? What makes the RH people less worthy to their code in than the SGI people. I'm convinced we're having this discussion for the right reasons, so let's let that line of discussion die where it is. Regards, Ben -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] update xfs maintainers
Hey Dave, On Mon, Nov 11, 2013 at 10:12:41AM +1100, Dave Chinner wrote: > On Sat, Nov 09, 2013 at 06:30:49PM -0600, Ben Myers wrote: > > On Sat, Nov 09, 2013 at 05:51:30PM -0600, Ben Myers wrote: > > > On Sat, Nov 09, 2013 at 10:44:24AM +1100, NeilBrown wrote: > > > > On Sat, 9 Nov 2013 06:59:00 +0800 Zhi Yong Wu > > > > wrote: > > > > > On Sat, Nov 9, 2013 at 6:03 AM, Ben Myers wrote: > > > > > > On Fri, Nov 08, 2013 at 03:50:21PM -0500, Ric Wheeler wrote: > > > > > >> On 11/08/2013 03:46 PM, Ben Myers wrote: > > > > > >> >Hey Christoph, > > > > > >> > > > > > > >> >On Fri, Nov 08, 2013 at 11:34:24AM -0800, Christoph Hellwig wrote: > > > > > >> >>On Fri, Nov 08, 2013 at 12:03:37PM -0600, Ben Myers wrote: > > > > > >> >>>Mark is replacing Alex as my backup because Alex is really busy > > > > > >> >>>at > > > > > >> >>>Linaro and asked to be taken off awhile ago. The holiday > > > > > >> >>>season is > > > > > >> >>>coming up and I fully intend to go off my meds, turn in to > > > > > >> >>>Fonzy the > > > > > >> >>>bear, and eat my hat. I need someone to watch the shop while > > > > > >> >>>I'm off > > > > > >> >>>exploring on Mars. I trust Mark to do that because he is > > > > > >> >>>totally > > > > > >> >>>awesome. > > > > > >> >> > > > > > >> >>Doing this as an unilateral decisions is not something that will > > > > > >> >>win you > > > > > >> >>a fan base. > > > > > >> >It's posted for review. > > > > > >> > > > > > > >> >>While we never had anything reassembling a democracy in Linux > > > > > >> >>Kernel > > > > > >> >>development making decisions without even contacting the major > > > > > >> >>contributor is wrong, twice so if the maintainer is a relatively > > > > > >> >>minor > > > > > >> >>contributor to start with. > > > > > >> >> > > > > > >> >>Just because it recent came up elsewhere I'd like to recite the > > > > > >> >>definition from Trond here again: > > > > > >> >> > > > > > >> >> > > > > > >> >> http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-June/66.html > > Yup, and my take on the role of a Maintainer is here: > > http://oss.sgi.com/archives/xfs/2013-08/msg00633.html > > > > > > > P: Silicon Graphics Inc > > > > > > +M: Dave Chinner > > I flattered by this, but there is a reason I've resisted taking this > position for a long time. Mainly for the reasons already mentioned: > > > > > Indeed. And does he even want the job? I heard Linus say in a recent > > > > interview that being a maintainer is a $#!+ job. > > > > > > I've found that it can be a little bit stressful sometimes and it tends to > > > crowd out feature work, so I guess I agree with him. It turns out to be > > > an > > > excellent weight loss plan. > > which are all true, but I'm already pulling most patches off the > list, applying them to my own trees, testing them and reviewing > them. Hence juggling them into a stable, non-rebasing git tree > branch on a server somewhere isn't a huge amount of extra work... > > > > > Is it really best for the > > > > most active developers to be burdened with that extra work? > > > > > > > > (hmm.. maybe I should add Dave to the Cc here .. but no-one else did so > > > > best > > > > leave him alone to code in peace). > > > > > > Dave, what do you want to do here? Which email? What sort of > > > arrangement? I > > > gather that you probably do want the job, and I know you'll be fantastic. > > > Do > > > you want to do it all yourself? Maybe split it up? > > > > I should have also suggested that we can add you to this file and just keep > > our > > existing arrangements. That seems appropriate to me, befitting of your > >
Re: [PATCH] update xfs maintainers
Hey Dave, On Mon, Nov 11, 2013 at 10:12:41AM +1100, Dave Chinner wrote: On Sat, Nov 09, 2013 at 06:30:49PM -0600, Ben Myers wrote: On Sat, Nov 09, 2013 at 05:51:30PM -0600, Ben Myers wrote: On Sat, Nov 09, 2013 at 10:44:24AM +1100, NeilBrown wrote: On Sat, 9 Nov 2013 06:59:00 +0800 Zhi Yong Wu zwu.ker...@gmail.com wrote: On Sat, Nov 9, 2013 at 6:03 AM, Ben Myers b...@sgi.com wrote: On Fri, Nov 08, 2013 at 03:50:21PM -0500, Ric Wheeler wrote: On 11/08/2013 03:46 PM, Ben Myers wrote: Hey Christoph, On Fri, Nov 08, 2013 at 11:34:24AM -0800, Christoph Hellwig wrote: On Fri, Nov 08, 2013 at 12:03:37PM -0600, Ben Myers wrote: Mark is replacing Alex as my backup because Alex is really busy at Linaro and asked to be taken off awhile ago. The holiday season is coming up and I fully intend to go off my meds, turn in to Fonzy the bear, and eat my hat. I need someone to watch the shop while I'm off exploring on Mars. I trust Mark to do that because he is totally awesome. Doing this as an unilateral decisions is not something that will win you a fan base. It's posted for review. While we never had anything reassembling a democracy in Linux Kernel development making decisions without even contacting the major contributor is wrong, twice so if the maintainer is a relatively minor contributor to start with. Just because it recent came up elsewhere I'd like to recite the definition from Trond here again: http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-June/66.html Yup, and my take on the role of a Maintainer is here: http://oss.sgi.com/archives/xfs/2013-08/msg00633.html P: Silicon Graphics Inc +M: Dave Chinner dchin...@fromorbit.com I flattered by this, but there is a reason I've resisted taking this position for a long time. Mainly for the reasons already mentioned: Indeed. And does he even want the job? I heard Linus say in a recent interview that being a maintainer is a $#!+ job. I've found that it can be a little bit stressful sometimes and it tends to crowd out feature work, so I guess I agree with him. It turns out to be an excellent weight loss plan. which are all true, but I'm already pulling most patches off the list, applying them to my own trees, testing them and reviewing them. Hence juggling them into a stable, non-rebasing git tree branch on a server somewhere isn't a huge amount of extra work... Is it really best for the most active developers to be burdened with that extra work? (hmm.. maybe I should add Dave to the Cc here .. but no-one else did so best leave him alone to code in peace). Dave, what do you want to do here? Which email? What sort of arrangement? I gather that you probably do want the job, and I know you'll be fantastic. Do you want to do it all yourself? Maybe split it up? I should have also suggested that we can add you to this file and just keep our existing arrangements. That seems appropriate to me, befitting of your achievements, the work you've been doing, and I'm willing to keep on as I am. OK, I've read the thread and had a think about it. My thoughts are as follows I'm not interested in being a co-maintainer in name only or only as a backup to only be used when Ben goes on holidays. Co-maintainer means SGI is giving me all the access and admin rights needed to commit and maintain git trees on oss.sgi.com (including creating new trees) at any time, at any point in a release cycle, etc. i.e. I can do anything that Ben can currently do on oss.sgi.com from XFS POV... Co-maintainer is not a kernel-tree only deal. It's for everything XFS related: kernel code, xfsprogs, xfstests and xfsdump. Co-maintainer does not mean Dave does everything. Yes, I can do a lot of the heavy lifting, but I'm very happy for Ben to continue committing patches he reviews and handling userspace releases and pushing stuff to Linus and so on. There's some logistics we need to work out here so we aren't going to step on each other's toes, but there's no unsolvable issues here. Co-maintainer is not a role I will perform with a Red Hat on. I will review and sign off on anything in my co-maintainer role as da...@fromorbit.com. Hence I hope to be able to maintain a clear distinction between the duties I perform on behalf of the community and code that I write on Red Hat's behalf. Lastly, being a maintainer doesn't solve the problem of review latency of the code I write. I'm hoping that everyone understands that maintainers are still dependent on the receiving help from the community they serve to get their own work done. Ben, let's talk more about
Re: [PATCH] update xfs maintainers
Dave, On Sat, Nov 09, 2013 at 05:51:30PM -0600, Ben Myers wrote: > Hey Neil, > > On Sat, Nov 09, 2013 at 10:44:24AM +1100, NeilBrown wrote: > > On Sat, 9 Nov 2013 06:59:00 +0800 Zhi Yong Wu wrote: > > > > > On Sat, Nov 9, 2013 at 6:03 AM, Ben Myers wrote: > > > > Hey Ric, > > > > > > > > On Fri, Nov 08, 2013 at 03:50:21PM -0500, Ric Wheeler wrote: > > > >> On 11/08/2013 03:46 PM, Ben Myers wrote: > > > >> >Hey Christoph, > > > >> > > > > >> >On Fri, Nov 08, 2013 at 11:34:24AM -0800, Christoph Hellwig wrote: > > > >> >>On Fri, Nov 08, 2013 at 12:03:37PM -0600, Ben Myers wrote: > > > >> >>>Mark is replacing Alex as my backup because Alex is really busy at > > > >> >>>Linaro and asked to be taken off awhile ago. The holiday season is > > > >> >>>coming up and I fully intend to go off my meds, turn in to Fonzy the > > > >> >>>bear, and eat my hat. I need someone to watch the shop while I'm > > > >> >>>off > > > >> >>>exploring on Mars. I trust Mark to do that because he is totally > > > >> >>>awesome. > > > >> >> > > > >> >>Doing this as an unilateral decisions is not something that will win > > > >> >>you > > > >> >>a fan base. > > > >> >It's posted for review. > > > >> > > > > >> >>While we never had anything reassembling a democracy in Linux Kernel > > > >> >>development making decisions without even contacting the major > > > >> >>contributor is wrong, twice so if the maintainer is a relatively > > > >> >>minor > > > >> >>contributor to start with. > > > >> >> > > > >> >>Just because it recent came up elsewhere I'd like to recite the > > > >> >>definition from Trond here again: > > > >> >> > > > >> >> > > > >> >> http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-June/66.html > > > >> >> > > > >> >>By many of the creative roles enlisted there it's clear that Dave > > > >> >>should > > > >> >>be the maintainer. He's been the main contributor and chief > > > >> >>architect > > > >> >>for XFS for many year, while the maintainers came and went at the > > > >> >>mercy > > > >> >>of SGI. This is not meant to bad mouth either of you as I think > > > >> >>you're > > > >> >>doing a reasonably good job compared to other maintainers, but at the > > > >> >>same time the direction is set by other people that have a much > > > >> >>longer > > > >> >>involvement with the project, and having them officially in control > > > >> >>would help us forward a lot. It would also avoid having to spend > > > >> >>considerable resources to train every new generation of SGI > > > >> >>maintainer. > > > >> >> > > > >> >>Coming to and end I would like to maintain Dave Chinner as the > > > >> >>primary > > > >> >>XFS maintainer for all the work he has done as biggest contributor > > > >> >>and > > > >> >>architect of XFS since longer than I can remember, and I would love > > > >> >>to > > > >> >>retain Ben Myers as a co-maintainer for all the good work he has done > > > >> >>maintaining and reviewing patches since November 2011. > > > >> >I think we're doing a decent job too. So thanks for that much at > > > >> >least. ;) > > > >> >>I would also like to use this post as a public venue to condemn the > > > >> >>unilateral smokey backroom decisions about XFS maintainership that > > > >> >>SGI is > > > >> >>trying to enforce on the community. > > > >> >That really didn't happen Christoph. It's not in my tree or in a > > > >> >pull request. > > > >> > > > > >> >Linus, let me know what you want to do. I do think we're doing a > > > >> >fair job over > > > >> >here, and (geez)
Re: [PATCH] update xfs maintainers
Hey Neil, On Sat, Nov 09, 2013 at 10:44:24AM +1100, NeilBrown wrote: > On Sat, 9 Nov 2013 06:59:00 +0800 Zhi Yong Wu wrote: > > > On Sat, Nov 9, 2013 at 6:03 AM, Ben Myers wrote: > > > Hey Ric, > > > > > > On Fri, Nov 08, 2013 at 03:50:21PM -0500, Ric Wheeler wrote: > > >> On 11/08/2013 03:46 PM, Ben Myers wrote: > > >> >Hey Christoph, > > >> > > > >> >On Fri, Nov 08, 2013 at 11:34:24AM -0800, Christoph Hellwig wrote: > > >> >>On Fri, Nov 08, 2013 at 12:03:37PM -0600, Ben Myers wrote: > > >> >>>Mark is replacing Alex as my backup because Alex is really busy at > > >> >>>Linaro and asked to be taken off awhile ago. The holiday season is > > >> >>>coming up and I fully intend to go off my meds, turn in to Fonzy the > > >> >>>bear, and eat my hat. I need someone to watch the shop while I'm off > > >> >>>exploring on Mars. I trust Mark to do that because he is totally > > >> >>>awesome. > > >> >> > > >> >>Doing this as an unilateral decisions is not something that will win > > >> >>you > > >> >>a fan base. > > >> >It's posted for review. > > >> > > > >> >>While we never had anything reassembling a democracy in Linux Kernel > > >> >>development making decisions without even contacting the major > > >> >>contributor is wrong, twice so if the maintainer is a relatively minor > > >> >>contributor to start with. > > >> >> > > >> >>Just because it recent came up elsewhere I'd like to recite the > > >> >>definition from Trond here again: > > >> >> > > >> >> > > >> >> http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-June/66.html > > >> >> > > >> >>By many of the creative roles enlisted there it's clear that Dave > > >> >>should > > >> >>be the maintainer. He's been the main contributor and chief architect > > >> >>for XFS for many year, while the maintainers came and went at the mercy > > >> >>of SGI. This is not meant to bad mouth either of you as I think you're > > >> >>doing a reasonably good job compared to other maintainers, but at the > > >> >>same time the direction is set by other people that have a much longer > > >> >>involvement with the project, and having them officially in control > > >> >>would help us forward a lot. It would also avoid having to spend > > >> >>considerable resources to train every new generation of SGI maintainer. > > >> >> > > >> >>Coming to and end I would like to maintain Dave Chinner as the primary > > >> >>XFS maintainer for all the work he has done as biggest contributor and > > >> >>architect of XFS since longer than I can remember, and I would love to > > >> >>retain Ben Myers as a co-maintainer for all the good work he has done > > >> >>maintaining and reviewing patches since November 2011. > > >> >I think we're doing a decent job too. So thanks for that much at > > >> >least. ;) > > >> >>I would also like to use this post as a public venue to condemn the > > >> >>unilateral smokey backroom decisions about XFS maintainership that SGI > > >> >>is > > >> >>trying to enforce on the community. > > >> >That really didn't happen Christoph. It's not in my tree or in a pull > > >> >request. > > >> > > > >> >Linus, let me know what you want to do. I do think we're doing a fair > > >> >job over > > >> >here, and (geez) I'm just trying to add Mark as my backup since Alex is > > >> >too > > >> >busy. I know the RH people want more control, and that's > > >> >understandable, but > > >> >they really don't need to replace me to get their code in. Ouch. > > >> > > > >> >Thanks, > > >> > Ben > > >> > > >> Christoph is not a Red Hat person. > > >> > > >> Jeff is from Oracle. > > >> > > >> This is not a Red Hat vs SGI thing, > > > > > > Sorry if my read on that was wrong. > > > > > >> Dave simply has earned the right > >
Re: [PATCH] update xfs maintainers
On Fri, Nov 08, 2013 at 06:32:33PM -0500, Ric Wheeler wrote: > On 11/08/2013 05:17 PM, Ben Myers wrote: > >Hey Ric, > > > >On Fri, Nov 08, 2013 at 05:07:45PM -0500, Ric Wheeler wrote: > >>On 11/08/2013 05:03 PM, Ben Myers wrote: > >>>Hey Ric, > >>> > >>>On Fri, Nov 08, 2013 at 03:50:21PM -0500, Ric Wheeler wrote: > >>>>On 11/08/2013 03:46 PM, Ben Myers wrote: > >>>>>Hey Christoph, > >>>>> > >>>>>On Fri, Nov 08, 2013 at 11:34:24AM -0800, Christoph Hellwig wrote: > >>>>>>On Fri, Nov 08, 2013 at 12:03:37PM -0600, Ben Myers wrote: > >>>>>>>Mark is replacing Alex as my backup because Alex is really busy at > >>>>>>>Linaro and asked to be taken off awhile ago. The holiday season is > >>>>>>>coming up and I fully intend to go off my meds, turn in to Fonzy the > >>>>>>>bear, and eat my hat. I need someone to watch the shop while I'm off > >>>>>>>exploring on Mars. I trust Mark to do that because he is totally > >>>>>>>awesome. > >>>>>>Doing this as an unilateral decisions is not something that will win you > >>>>>>a fan base. > >>>>>It's posted for review. > >>>>> > >>>>>>While we never had anything reassembling a democracy in Linux Kernel > >>>>>>development making decisions without even contacting the major > >>>>>>contributor is wrong, twice so if the maintainer is a relatively minor > >>>>>>contributor to start with. > >>>>>> > >>>>>>Just because it recent came up elsewhere I'd like to recite the > >>>>>>definition from Trond here again: > >>>>>> > >>>>>> > >>>>>> http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-June/66.html > >>>>>> > >>>>>>By many of the creative roles enlisted there it's clear that Dave should > >>>>>>be the maintainer. He's been the main contributor and chief architect > >>>>>>for XFS for many year, while the maintainers came and went at the mercy > >>>>>>of SGI. This is not meant to bad mouth either of you as I think you're > >>>>>>doing a reasonably good job compared to other maintainers, but at the > >>>>>>same time the direction is set by other people that have a much longer > >>>>>>involvement with the project, and having them officially in control > >>>>>>would help us forward a lot. It would also avoid having to spend > >>>>>>considerable resources to train every new generation of SGI maintainer. > >>>>>> > >>>>>>Coming to and end I would like to maintain Dave Chinner as the primary > >>>>>>XFS maintainer for all the work he has done as biggest contributor and > >>>>>>architect of XFS since longer than I can remember, and I would love to > >>>>>>retain Ben Myers as a co-maintainer for all the good work he has done > >>>>>>maintaining and reviewing patches since November 2011. > >>>>>I think we're doing a decent job too. So thanks for that much at least. > >>>>> ;) > >>>>>>I would also like to use this post as a public venue to condemn the > >>>>>>unilateral smokey backroom decisions about XFS maintainership that SGI > >>>>>>is > >>>>>>trying to enforce on the community. > >>>>>That really didn't happen Christoph. It's not in my tree or in a pull > >>>>>request. > >>>>> > >>>>>Linus, let me know what you want to do. I do think we're doing a fair > >>>>>job over > >>>>>here, and (geez) I'm just trying to add Mark as my backup since Alex is > >>>>>too > >>>>>busy. I know the RH people want more control, and that's > >>>>>understandable, but > >>>>>they really don't need to replace me to get their code in. Ouch. > >>>>> > >>>>>Thanks, > >>>>> Ben > >>>>Christoph is not a Red Hat person. > >>>> > >>>>Jeff is from Oracle. > >>>> > >>>>This is not a Red Hat vs SGI thing, > >>>Sorry if my read on that was wrong. > >>I do appreciate the work and effort you and the SGI team put in but > >>think that this will be a good way to keep the community happier and > >>even more productive going forward. > >> > >>>>Dave simply has earned the right > >>>>to take on the formal leadership role of maintainer. > >>>Then we're gonna need some Reviewed-bys. ;) > >>Those should come from the developers, thanks! > >I actually do need your Reviewed-by. We'll try and get this one in 3.13. > >;) > > > >Thanks, > > Ben > > Happy to do that - I do think that Dave mostly posts from his > redhat.com account, but he can comment once he gets back online. > > Reviewed-by: Ric Wheeler Thanks Ric. ;) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] update xfs maintainers
On Fri, Nov 08, 2013 at 06:32:33PM -0500, Ric Wheeler wrote: On 11/08/2013 05:17 PM, Ben Myers wrote: Hey Ric, On Fri, Nov 08, 2013 at 05:07:45PM -0500, Ric Wheeler wrote: On 11/08/2013 05:03 PM, Ben Myers wrote: Hey Ric, On Fri, Nov 08, 2013 at 03:50:21PM -0500, Ric Wheeler wrote: On 11/08/2013 03:46 PM, Ben Myers wrote: Hey Christoph, On Fri, Nov 08, 2013 at 11:34:24AM -0800, Christoph Hellwig wrote: On Fri, Nov 08, 2013 at 12:03:37PM -0600, Ben Myers wrote: Mark is replacing Alex as my backup because Alex is really busy at Linaro and asked to be taken off awhile ago. The holiday season is coming up and I fully intend to go off my meds, turn in to Fonzy the bear, and eat my hat. I need someone to watch the shop while I'm off exploring on Mars. I trust Mark to do that because he is totally awesome. Doing this as an unilateral decisions is not something that will win you a fan base. It's posted for review. While we never had anything reassembling a democracy in Linux Kernel development making decisions without even contacting the major contributor is wrong, twice so if the maintainer is a relatively minor contributor to start with. Just because it recent came up elsewhere I'd like to recite the definition from Trond here again: http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-June/66.html By many of the creative roles enlisted there it's clear that Dave should be the maintainer. He's been the main contributor and chief architect for XFS for many year, while the maintainers came and went at the mercy of SGI. This is not meant to bad mouth either of you as I think you're doing a reasonably good job compared to other maintainers, but at the same time the direction is set by other people that have a much longer involvement with the project, and having them officially in control would help us forward a lot. It would also avoid having to spend considerable resources to train every new generation of SGI maintainer. Coming to and end I would like to maintain Dave Chinner as the primary XFS maintainer for all the work he has done as biggest contributor and architect of XFS since longer than I can remember, and I would love to retain Ben Myers as a co-maintainer for all the good work he has done maintaining and reviewing patches since November 2011. I think we're doing a decent job too. So thanks for that much at least. ;) I would also like to use this post as a public venue to condemn the unilateral smokey backroom decisions about XFS maintainership that SGI is trying to enforce on the community. That really didn't happen Christoph. It's not in my tree or in a pull request. Linus, let me know what you want to do. I do think we're doing a fair job over here, and (geez) I'm just trying to add Mark as my backup since Alex is too busy. I know the RH people want more control, and that's understandable, but they really don't need to replace me to get their code in. Ouch. Thanks, Ben Christoph is not a Red Hat person. Jeff is from Oracle. This is not a Red Hat vs SGI thing, Sorry if my read on that was wrong. I do appreciate the work and effort you and the SGI team put in but think that this will be a good way to keep the community happier and even more productive going forward. Dave simply has earned the right to take on the formal leadership role of maintainer. Then we're gonna need some Reviewed-bys. ;) Those should come from the developers, thanks! I actually do need your Reviewed-by. We'll try and get this one in 3.13. ;) Thanks, Ben Happy to do that - I do think that Dave mostly posts from his redhat.com account, but he can comment once he gets back online. Reviewed-by: Ric Wheeler rwhee...@redhat.com Thanks Ric. ;) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] update xfs maintainers
Hey Neil, On Sat, Nov 09, 2013 at 10:44:24AM +1100, NeilBrown wrote: On Sat, 9 Nov 2013 06:59:00 +0800 Zhi Yong Wu zwu.ker...@gmail.com wrote: On Sat, Nov 9, 2013 at 6:03 AM, Ben Myers b...@sgi.com wrote: Hey Ric, On Fri, Nov 08, 2013 at 03:50:21PM -0500, Ric Wheeler wrote: On 11/08/2013 03:46 PM, Ben Myers wrote: Hey Christoph, On Fri, Nov 08, 2013 at 11:34:24AM -0800, Christoph Hellwig wrote: On Fri, Nov 08, 2013 at 12:03:37PM -0600, Ben Myers wrote: Mark is replacing Alex as my backup because Alex is really busy at Linaro and asked to be taken off awhile ago. The holiday season is coming up and I fully intend to go off my meds, turn in to Fonzy the bear, and eat my hat. I need someone to watch the shop while I'm off exploring on Mars. I trust Mark to do that because he is totally awesome. Doing this as an unilateral decisions is not something that will win you a fan base. It's posted for review. While we never had anything reassembling a democracy in Linux Kernel development making decisions without even contacting the major contributor is wrong, twice so if the maintainer is a relatively minor contributor to start with. Just because it recent came up elsewhere I'd like to recite the definition from Trond here again: http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-June/66.html By many of the creative roles enlisted there it's clear that Dave should be the maintainer. He's been the main contributor and chief architect for XFS for many year, while the maintainers came and went at the mercy of SGI. This is not meant to bad mouth either of you as I think you're doing a reasonably good job compared to other maintainers, but at the same time the direction is set by other people that have a much longer involvement with the project, and having them officially in control would help us forward a lot. It would also avoid having to spend considerable resources to train every new generation of SGI maintainer. Coming to and end I would like to maintain Dave Chinner as the primary XFS maintainer for all the work he has done as biggest contributor and architect of XFS since longer than I can remember, and I would love to retain Ben Myers as a co-maintainer for all the good work he has done maintaining and reviewing patches since November 2011. I think we're doing a decent job too. So thanks for that much at least. ;) I would also like to use this post as a public venue to condemn the unilateral smokey backroom decisions about XFS maintainership that SGI is trying to enforce on the community. That really didn't happen Christoph. It's not in my tree or in a pull request. Linus, let me know what you want to do. I do think we're doing a fair job over here, and (geez) I'm just trying to add Mark as my backup since Alex is too busy. I know the RH people want more control, and that's understandable, but they really don't need to replace me to get their code in. Ouch. Thanks, Ben Christoph is not a Red Hat person. Jeff is from Oracle. This is not a Red Hat vs SGI thing, Sorry if my read on that was wrong. Dave simply has earned the right to take on the formal leadership role of maintainer. Then we're gonna need some Reviewed-bys. ;) From: Ben Myers b...@sgi.com xfs: update maintainers Add Dave as maintainer of XFS. Signed-off-by: Ben Myers b...@sgi.com --- MAINTAINERS |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: b/MAINTAINERS === --- a/MAINTAINERS 2013-11-08 15:20:18.935186245 -0600 +++ b/MAINTAINERS 2013-11-08 15:22:50.685245977 -0600 @@ -9387,8 +9387,8 @@ F:drivers/xen/*swiotlb* XFS FILESYSTEM P: Silicon Graphics Inc +M: Dave Chinner dchin...@fromorbit.com Use his personal private mail account? I guess that you should ask for his opinion at first, or it is more appropriate that he submit this patch by himself. If y'all don't mind, I'd like to have authored this one. ;) Indeed. And does he even want the job? I heard Linus say in a recent interview that being a maintainer is a $#!+ job. I've found that it can be a little bit stressful sometimes and it tends to crowd out feature work, so I guess I agree with him. It turns out to be an excellent weight loss plan. Is it really best for the most active developers to be burdened with that extra work? (hmm.. maybe I should add Dave to the Cc here .. but no-one else did so best leave him alone to code in peace). Dave, what do you want to do here? Which email? What sort of arrangement? I gather that you probably do want the job, and I know you'll be fantastic. Do you want to do
Re: [PATCH] update xfs maintainers
Dave, On Sat, Nov 09, 2013 at 05:51:30PM -0600, Ben Myers wrote: Hey Neil, On Sat, Nov 09, 2013 at 10:44:24AM +1100, NeilBrown wrote: On Sat, 9 Nov 2013 06:59:00 +0800 Zhi Yong Wu zwu.ker...@gmail.com wrote: On Sat, Nov 9, 2013 at 6:03 AM, Ben Myers b...@sgi.com wrote: Hey Ric, On Fri, Nov 08, 2013 at 03:50:21PM -0500, Ric Wheeler wrote: On 11/08/2013 03:46 PM, Ben Myers wrote: Hey Christoph, On Fri, Nov 08, 2013 at 11:34:24AM -0800, Christoph Hellwig wrote: On Fri, Nov 08, 2013 at 12:03:37PM -0600, Ben Myers wrote: Mark is replacing Alex as my backup because Alex is really busy at Linaro and asked to be taken off awhile ago. The holiday season is coming up and I fully intend to go off my meds, turn in to Fonzy the bear, and eat my hat. I need someone to watch the shop while I'm off exploring on Mars. I trust Mark to do that because he is totally awesome. Doing this as an unilateral decisions is not something that will win you a fan base. It's posted for review. While we never had anything reassembling a democracy in Linux Kernel development making decisions without even contacting the major contributor is wrong, twice so if the maintainer is a relatively minor contributor to start with. Just because it recent came up elsewhere I'd like to recite the definition from Trond here again: http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-June/66.html By many of the creative roles enlisted there it's clear that Dave should be the maintainer. He's been the main contributor and chief architect for XFS for many year, while the maintainers came and went at the mercy of SGI. This is not meant to bad mouth either of you as I think you're doing a reasonably good job compared to other maintainers, but at the same time the direction is set by other people that have a much longer involvement with the project, and having them officially in control would help us forward a lot. It would also avoid having to spend considerable resources to train every new generation of SGI maintainer. Coming to and end I would like to maintain Dave Chinner as the primary XFS maintainer for all the work he has done as biggest contributor and architect of XFS since longer than I can remember, and I would love to retain Ben Myers as a co-maintainer for all the good work he has done maintaining and reviewing patches since November 2011. I think we're doing a decent job too. So thanks for that much at least. ;) I would also like to use this post as a public venue to condemn the unilateral smokey backroom decisions about XFS maintainership that SGI is trying to enforce on the community. That really didn't happen Christoph. It's not in my tree or in a pull request. Linus, let me know what you want to do. I do think we're doing a fair job over here, and (geez) I'm just trying to add Mark as my backup since Alex is too busy. I know the RH people want more control, and that's understandable, but they really don't need to replace me to get their code in. Ouch. Thanks, Ben Christoph is not a Red Hat person. Jeff is from Oracle. This is not a Red Hat vs SGI thing, Sorry if my read on that was wrong. Dave simply has earned the right to take on the formal leadership role of maintainer. Then we're gonna need some Reviewed-bys. ;) From: Ben Myers b...@sgi.com xfs: update maintainers Add Dave as maintainer of XFS. Signed-off-by: Ben Myers b...@sgi.com --- MAINTAINERS |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: b/MAINTAINERS === --- a/MAINTAINERS 2013-11-08 15:20:18.935186245 -0600 +++ b/MAINTAINERS 2013-11-08 15:22:50.685245977 -0600 @@ -9387,8 +9387,8 @@ F:drivers/xen/*swiotlb* XFS FILESYSTEM P: Silicon Graphics Inc +M: Dave Chinner dchin...@fromorbit.com Use his personal private mail account? I guess that you should ask for his opinion at first, or it is more appropriate that he submit this patch by himself. If y'all don't mind, I'd like to have authored this one. ;) Indeed. And does he even want the job? I heard Linus say in a recent interview that being a maintainer is a $#!+ job. I've found that it can be a little bit stressful sometimes and it tends to crowd out feature work, so I guess I agree with him. It turns out to be an excellent weight loss plan. Is it really best for the most active developers to be burdened with that extra work? (hmm.. maybe I should add Dave to the Cc here
Re: [PATCH] update xfs maintainers
Hey Ric, On Fri, Nov 08, 2013 at 05:07:45PM -0500, Ric Wheeler wrote: > On 11/08/2013 05:03 PM, Ben Myers wrote: > >Hey Ric, > > > >On Fri, Nov 08, 2013 at 03:50:21PM -0500, Ric Wheeler wrote: > >>On 11/08/2013 03:46 PM, Ben Myers wrote: > >>>Hey Christoph, > >>> > >>>On Fri, Nov 08, 2013 at 11:34:24AM -0800, Christoph Hellwig wrote: > >>>>On Fri, Nov 08, 2013 at 12:03:37PM -0600, Ben Myers wrote: > >>>>>Mark is replacing Alex as my backup because Alex is really busy at > >>>>>Linaro and asked to be taken off awhile ago. The holiday season is > >>>>>coming up and I fully intend to go off my meds, turn in to Fonzy the > >>>>>bear, and eat my hat. I need someone to watch the shop while I'm off > >>>>>exploring on Mars. I trust Mark to do that because he is totally > >>>>>awesome. > >>>>Doing this as an unilateral decisions is not something that will win you > >>>>a fan base. > >>>It's posted for review. > >>> > >>>>While we never had anything reassembling a democracy in Linux Kernel > >>>>development making decisions without even contacting the major > >>>>contributor is wrong, twice so if the maintainer is a relatively minor > >>>>contributor to start with. > >>>> > >>>>Just because it recent came up elsewhere I'd like to recite the > >>>>definition from Trond here again: > >>>> > >>>> > >>>> http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-June/66.html > >>>> > >>>>By many of the creative roles enlisted there it's clear that Dave should > >>>>be the maintainer. He's been the main contributor and chief architect > >>>>for XFS for many year, while the maintainers came and went at the mercy > >>>>of SGI. This is not meant to bad mouth either of you as I think you're > >>>>doing a reasonably good job compared to other maintainers, but at the > >>>>same time the direction is set by other people that have a much longer > >>>>involvement with the project, and having them officially in control > >>>>would help us forward a lot. It would also avoid having to spend > >>>>considerable resources to train every new generation of SGI maintainer. > >>>> > >>>>Coming to and end I would like to maintain Dave Chinner as the primary > >>>>XFS maintainer for all the work he has done as biggest contributor and > >>>>architect of XFS since longer than I can remember, and I would love to > >>>>retain Ben Myers as a co-maintainer for all the good work he has done > >>>>maintaining and reviewing patches since November 2011. > >>>I think we're doing a decent job too. So thanks for that much at least. > >>>;) > >>>>I would also like to use this post as a public venue to condemn the > >>>>unilateral smokey backroom decisions about XFS maintainership that SGI is > >>>>trying to enforce on the community. > >>>That really didn't happen Christoph. It's not in my tree or in a pull > >>>request. > >>> > >>>Linus, let me know what you want to do. I do think we're doing a fair job > >>>over > >>>here, and (geez) I'm just trying to add Mark as my backup since Alex is too > >>>busy. I know the RH people want more control, and that's understandable, > >>>but > >>>they really don't need to replace me to get their code in. Ouch. > >>> > >>>Thanks, > >>> Ben > >>Christoph is not a Red Hat person. > >> > >>Jeff is from Oracle. > >> > >>This is not a Red Hat vs SGI thing, > >Sorry if my read on that was wrong. > > I do appreciate the work and effort you and the SGI team put in but > think that this will be a good way to keep the community happier and > even more productive going forward. > > >>Dave simply has earned the right > >>to take on the formal leadership role of maintainer. > >Then we're gonna need some Reviewed-bys. ;) > > Those should come from the developers, thanks! I actually do need your Reviewed-by. We'll try and get this one in 3.13. ;) Thanks, Ben > >From: Ben Myers > > > >xfs: update maintainers > > > >Add Dave as maintainer of XFS. > > > >Signed-off-by: Ben Myers > >--- > > MAINTAINERS |2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > >Index: b/MAINTAINERS > >=== > >--- a/MAINTAINERS2013-11-08 15:20:18.935186245 -0600 > >+++ b/MAINTAINERS2013-11-08 15:22:50.685245977 -0600 > >@@ -9387,8 +9387,8 @@ F: drivers/xen/*swiotlb* > > XFS FILESYSTEM > > P: Silicon Graphics Inc > >+M: Dave Chinner > > M: Ben Myers > >-M: Alex Elder > > M: x...@oss.sgi.com > > L: x...@oss.sgi.com > > W: http://oss.sgi.com/projects/xfs > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] update xfs maintainers
Hey Ric, On Fri, Nov 08, 2013 at 03:50:21PM -0500, Ric Wheeler wrote: > On 11/08/2013 03:46 PM, Ben Myers wrote: > >Hey Christoph, > > > >On Fri, Nov 08, 2013 at 11:34:24AM -0800, Christoph Hellwig wrote: > >>On Fri, Nov 08, 2013 at 12:03:37PM -0600, Ben Myers wrote: > >>>Mark is replacing Alex as my backup because Alex is really busy at > >>>Linaro and asked to be taken off awhile ago. The holiday season is > >>>coming up and I fully intend to go off my meds, turn in to Fonzy the > >>>bear, and eat my hat. I need someone to watch the shop while I'm off > >>>exploring on Mars. I trust Mark to do that because he is totally > >>>awesome. > >> > >>Doing this as an unilateral decisions is not something that will win you > >>a fan base. > >It's posted for review. > > > >>While we never had anything reassembling a democracy in Linux Kernel > >>development making decisions without even contacting the major > >>contributor is wrong, twice so if the maintainer is a relatively minor > >>contributor to start with. > >> > >>Just because it recent came up elsewhere I'd like to recite the > >>definition from Trond here again: > >> > >> > >> http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-June/66.html > >> > >>By many of the creative roles enlisted there it's clear that Dave should > >>be the maintainer. He's been the main contributor and chief architect > >>for XFS for many year, while the maintainers came and went at the mercy > >>of SGI. This is not meant to bad mouth either of you as I think you're > >>doing a reasonably good job compared to other maintainers, but at the > >>same time the direction is set by other people that have a much longer > >>involvement with the project, and having them officially in control > >>would help us forward a lot. It would also avoid having to spend > >>considerable resources to train every new generation of SGI maintainer. > >> > >>Coming to and end I would like to maintain Dave Chinner as the primary > >>XFS maintainer for all the work he has done as biggest contributor and > >>architect of XFS since longer than I can remember, and I would love to > >>retain Ben Myers as a co-maintainer for all the good work he has done > >>maintaining and reviewing patches since November 2011. > >I think we're doing a decent job too. So thanks for that much at least. ;) > >>I would also like to use this post as a public venue to condemn the > >>unilateral smokey backroom decisions about XFS maintainership that SGI is > >>trying to enforce on the community. > >That really didn't happen Christoph. It's not in my tree or in a pull > >request. > > > >Linus, let me know what you want to do. I do think we're doing a fair job > >over > >here, and (geez) I'm just trying to add Mark as my backup since Alex is too > >busy. I know the RH people want more control, and that's understandable, but > >they really don't need to replace me to get their code in. Ouch. > > > >Thanks, > > Ben > > Christoph is not a Red Hat person. > > Jeff is from Oracle. > > This is not a Red Hat vs SGI thing, Sorry if my read on that was wrong. > Dave simply has earned the right > to take on the formal leadership role of maintainer. Then we're gonna need some Reviewed-bys. ;) From: Ben Myers xfs: update maintainers Add Dave as maintainer of XFS. Signed-off-by: Ben Myers --- MAINTAINERS |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: b/MAINTAINERS === --- a/MAINTAINERS 2013-11-08 15:20:18.935186245 -0600 +++ b/MAINTAINERS 2013-11-08 15:22:50.685245977 -0600 @@ -9387,8 +9387,8 @@ F:drivers/xen/*swiotlb* XFS FILESYSTEM P: Silicon Graphics Inc +M: Dave Chinner M: Ben Myers -M: Alex Elder M: x...@oss.sgi.com L: x...@oss.sgi.com W: http://oss.sgi.com/projects/xfs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: XFS leadership and a new co-maintainer candidate
Hey Christoph, On Fri, Nov 08, 2013 at 11:34:24AM -0800, Christoph Hellwig wrote: > On Fri, Nov 08, 2013 at 12:03:37PM -0600, Ben Myers wrote: > > Mark is replacing Alex as my backup because Alex is really busy at > > Linaro and asked to be taken off awhile ago. The holiday season is > > coming up and I fully intend to go off my meds, turn in to Fonzy the > > bear, and eat my hat. I need someone to watch the shop while I'm off > > exploring on Mars. I trust Mark to do that because he is totally > > awesome. > > > Doing this as an unilateral decisions is not something that will win you > a fan base. It's posted for review. > While we never had anything reassembling a democracy in Linux Kernel > development making decisions without even contacting the major > contributor is wrong, twice so if the maintainer is a relatively minor > contributor to start with. > > Just because it recent came up elsewhere I'd like to recite the > definition from Trond here again: > > > http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-June/66.html > > By many of the creative roles enlisted there it's clear that Dave should > be the maintainer. He's been the main contributor and chief architect > for XFS for many year, while the maintainers came and went at the mercy > of SGI. This is not meant to bad mouth either of you as I think you're > doing a reasonably good job compared to other maintainers, but at the > same time the direction is set by other people that have a much longer > involvement with the project, and having them officially in control > would help us forward a lot. It would also avoid having to spend > considerable resources to train every new generation of SGI maintainer. > > Coming to and end I would like to maintain Dave Chinner as the primary > XFS maintainer for all the work he has done as biggest contributor and > architect of XFS since longer than I can remember, and I would love to > retain Ben Myers as a co-maintainer for all the good work he has done > maintaining and reviewing patches since November 2011. I think we're doing a decent job too. So thanks for that much at least. ;) > I would also like to use this post as a public venue to condemn the > unilateral smokey backroom decisions about XFS maintainership that SGI is > trying to enforce on the community. That really didn't happen Christoph. It's not in my tree or in a pull request. Linus, let me know what you want to do. I do think we're doing a fair job over here, and (geez) I'm just trying to add Mark as my backup since Alex is too busy. I know the RH people want more control, and that's understandable, but they really don't need to replace me to get their code in. Ouch. Thanks, Ben -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: XFS leadership and a new co-maintainer candidate
Hey Ric, On Fri, Nov 08, 2013 at 01:09:32PM -0500, Ric Wheeler wrote: > On 11/08/2013 01:03 PM, Ben Myers wrote: > >Hey Ric, > > > >On Fri, Nov 08, 2013 at 06:03:41AM -0500, Ric Wheeler wrote: > >>In the XFS community, we have 2 clear leaders in terms of > >>contributions of significant feaures and depth of knowledge - > >>Christoph and Dave. > >> > >>If you look at the number of patches submitted by developers since > >>3.0 who have more than 10 patches, we get the following: > >> > >> 319 Author: Dave Chinner > >> 163 Author: Christoph Hellwig > >> 51 Author: Christoph Hellwig > >> 35 Author: Linus Torvalds > >> 34 Author: Chandra Seetharaman > >> 29 Author: Al Viro > >> 28 Author: Brian Foster > >> 25 Author: Zhi Yong Wu > >> 24 Author: Jeff Liu > >> 21 Author: Jie Liu > >> 20 Author: Mark Tinguely > >> 16 Author: Dave Chinner > >> 12 Author: Eric Sandeen > >> 12 Author: Carlos Maiolino > >> > >>If we as a community had more capacity for patch review, Dave's > >>numbers would have jumped up even higher :) > >> > >>It is certainly very welcome to bring new developers into our > >>community, but if we are going to add a co-maintainer for XFS, we > >>really need to have one of our two leading developers in that role. > >Mark is replacing Alex as my backup because Alex is really busy at > >Linaro and asked to be taken off awhile ago. The holiday season is > >coming up and I fully intend to go off my meds, turn in to Fonzy the > >bear, and eat my hat. I need someone to watch the shop while I'm off > >exploring on Mars. I trust Mark to do that because he is totally > >awesome. > > > >-Ben > > I don't mean any disrepect to you or to Mark, Don't worry about it, I have plenty to spare. ;P > but maintainership is > something that you earn over time by proving yourself in the > community as a developer and a leader of the technology on a > personal level. > > It is not something that gets managed by the community of developers > and has the key role of keeping the most frequent developers engaged > and happy. That has not been working for us as a community lately. > > Dave Chinner is the obvious person to take on the maintainer role as > someone who has an order of magnitude more code contributed than > either of you (even combined). > > Christoph, if he has time, would also be an excellent candidate. Eric is also a good choice. I'd be happy to add all three. -Ben -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: XFS leadership and a new co-maintainer candidate
Hey Ric, On Fri, Nov 08, 2013 at 06:03:41AM -0500, Ric Wheeler wrote: > In the XFS community, we have 2 clear leaders in terms of > contributions of significant feaures and depth of knowledge - > Christoph and Dave. > > If you look at the number of patches submitted by developers since > 3.0 who have more than 10 patches, we get the following: > > 319 Author: Dave Chinner > 163 Author: Christoph Hellwig > 51 Author: Christoph Hellwig > 35 Author: Linus Torvalds > 34 Author: Chandra Seetharaman > 29 Author: Al Viro > 28 Author: Brian Foster > 25 Author: Zhi Yong Wu > 24 Author: Jeff Liu > 21 Author: Jie Liu > 20 Author: Mark Tinguely > 16 Author: Dave Chinner > 12 Author: Eric Sandeen > 12 Author: Carlos Maiolino > > If we as a community had more capacity for patch review, Dave's > numbers would have jumped up even higher :) > > It is certainly very welcome to bring new developers into our > community, but if we are going to add a co-maintainer for XFS, we > really need to have one of our two leading developers in that role. Mark is replacing Alex as my backup because Alex is really busy at Linaro and asked to be taken off awhile ago. The holiday season is coming up and I fully intend to go off my meds, turn in to Fonzy the bear, and eat my hat. I need someone to watch the shop while I'm off exploring on Mars. I trust Mark to do that because he is totally awesome. -Ben -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: XFS leadership and a new co-maintainer candidate
Hey Ric, On Fri, Nov 08, 2013 at 06:03:41AM -0500, Ric Wheeler wrote: In the XFS community, we have 2 clear leaders in terms of contributions of significant feaures and depth of knowledge - Christoph and Dave. If you look at the number of patches submitted by developers since 3.0 who have more than 10 patches, we get the following: 319 Author: Dave Chinner dchin...@redhat.com 163 Author: Christoph Hellwig h...@infradead.org 51 Author: Christoph Hellwig h...@lst.de 35 Author: Linus Torvalds torva...@linux-foundation.org 34 Author: Chandra Seetharaman sekha...@us.ibm.com 29 Author: Al Viro v...@zeniv.linux.org.uk 28 Author: Brian Foster bfos...@redhat.com 25 Author: Zhi Yong Wu wu...@linux.vnet.ibm.com 24 Author: Jeff Liu jeff@oracle.com 21 Author: Jie Liu jeff@oracle.com 20 Author: Mark Tinguely tingu...@sgi.com 16 Author: Dave Chinner da...@fromorbit.com 12 Author: Eric Sandeen sand...@redhat.com 12 Author: Carlos Maiolino cmaiol...@redhat.com If we as a community had more capacity for patch review, Dave's numbers would have jumped up even higher :) It is certainly very welcome to bring new developers into our community, but if we are going to add a co-maintainer for XFS, we really need to have one of our two leading developers in that role. Mark is replacing Alex as my backup because Alex is really busy at Linaro and asked to be taken off awhile ago. The holiday season is coming up and I fully intend to go off my meds, turn in to Fonzy the bear, and eat my hat. I need someone to watch the shop while I'm off exploring on Mars. I trust Mark to do that because he is totally awesome. -Ben -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: XFS leadership and a new co-maintainer candidate
Hey Ric, On Fri, Nov 08, 2013 at 01:09:32PM -0500, Ric Wheeler wrote: On 11/08/2013 01:03 PM, Ben Myers wrote: Hey Ric, On Fri, Nov 08, 2013 at 06:03:41AM -0500, Ric Wheeler wrote: In the XFS community, we have 2 clear leaders in terms of contributions of significant feaures and depth of knowledge - Christoph and Dave. If you look at the number of patches submitted by developers since 3.0 who have more than 10 patches, we get the following: 319 Author: Dave Chinner dchin...@redhat.com 163 Author: Christoph Hellwig h...@infradead.org 51 Author: Christoph Hellwig h...@lst.de 35 Author: Linus Torvalds torva...@linux-foundation.org 34 Author: Chandra Seetharaman sekha...@us.ibm.com 29 Author: Al Viro v...@zeniv.linux.org.uk 28 Author: Brian Foster bfos...@redhat.com 25 Author: Zhi Yong Wu wu...@linux.vnet.ibm.com 24 Author: Jeff Liu jeff@oracle.com 21 Author: Jie Liu jeff@oracle.com 20 Author: Mark Tinguely tingu...@sgi.com 16 Author: Dave Chinner da...@fromorbit.com 12 Author: Eric Sandeen sand...@redhat.com 12 Author: Carlos Maiolino cmaiol...@redhat.com If we as a community had more capacity for patch review, Dave's numbers would have jumped up even higher :) It is certainly very welcome to bring new developers into our community, but if we are going to add a co-maintainer for XFS, we really need to have one of our two leading developers in that role. Mark is replacing Alex as my backup because Alex is really busy at Linaro and asked to be taken off awhile ago. The holiday season is coming up and I fully intend to go off my meds, turn in to Fonzy the bear, and eat my hat. I need someone to watch the shop while I'm off exploring on Mars. I trust Mark to do that because he is totally awesome. -Ben I don't mean any disrepect to you or to Mark, Don't worry about it, I have plenty to spare. ;P but maintainership is something that you earn over time by proving yourself in the community as a developer and a leader of the technology on a personal level. It is not something that gets managed by the community of developers and has the key role of keeping the most frequent developers engaged and happy. That has not been working for us as a community lately. Dave Chinner is the obvious person to take on the maintainer role as someone who has an order of magnitude more code contributed than either of you (even combined). Christoph, if he has time, would also be an excellent candidate. Eric is also a good choice. I'd be happy to add all three. -Ben -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: XFS leadership and a new co-maintainer candidate
Hey Christoph, On Fri, Nov 08, 2013 at 11:34:24AM -0800, Christoph Hellwig wrote: On Fri, Nov 08, 2013 at 12:03:37PM -0600, Ben Myers wrote: Mark is replacing Alex as my backup because Alex is really busy at Linaro and asked to be taken off awhile ago. The holiday season is coming up and I fully intend to go off my meds, turn in to Fonzy the bear, and eat my hat. I need someone to watch the shop while I'm off exploring on Mars. I trust Mark to do that because he is totally awesome. Doing this as an unilateral decisions is not something that will win you a fan base. It's posted for review. While we never had anything reassembling a democracy in Linux Kernel development making decisions without even contacting the major contributor is wrong, twice so if the maintainer is a relatively minor contributor to start with. Just because it recent came up elsewhere I'd like to recite the definition from Trond here again: http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-June/66.html By many of the creative roles enlisted there it's clear that Dave should be the maintainer. He's been the main contributor and chief architect for XFS for many year, while the maintainers came and went at the mercy of SGI. This is not meant to bad mouth either of you as I think you're doing a reasonably good job compared to other maintainers, but at the same time the direction is set by other people that have a much longer involvement with the project, and having them officially in control would help us forward a lot. It would also avoid having to spend considerable resources to train every new generation of SGI maintainer. Coming to and end I would like to maintain Dave Chinner as the primary XFS maintainer for all the work he has done as biggest contributor and architect of XFS since longer than I can remember, and I would love to retain Ben Myers as a co-maintainer for all the good work he has done maintaining and reviewing patches since November 2011. I think we're doing a decent job too. So thanks for that much at least. ;) I would also like to use this post as a public venue to condemn the unilateral smokey backroom decisions about XFS maintainership that SGI is trying to enforce on the community. That really didn't happen Christoph. It's not in my tree or in a pull request. Linus, let me know what you want to do. I do think we're doing a fair job over here, and (geez) I'm just trying to add Mark as my backup since Alex is too busy. I know the RH people want more control, and that's understandable, but they really don't need to replace me to get their code in. Ouch. Thanks, Ben -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] update xfs maintainers
Hey Ric, On Fri, Nov 08, 2013 at 03:50:21PM -0500, Ric Wheeler wrote: On 11/08/2013 03:46 PM, Ben Myers wrote: Hey Christoph, On Fri, Nov 08, 2013 at 11:34:24AM -0800, Christoph Hellwig wrote: On Fri, Nov 08, 2013 at 12:03:37PM -0600, Ben Myers wrote: Mark is replacing Alex as my backup because Alex is really busy at Linaro and asked to be taken off awhile ago. The holiday season is coming up and I fully intend to go off my meds, turn in to Fonzy the bear, and eat my hat. I need someone to watch the shop while I'm off exploring on Mars. I trust Mark to do that because he is totally awesome. Doing this as an unilateral decisions is not something that will win you a fan base. It's posted for review. While we never had anything reassembling a democracy in Linux Kernel development making decisions without even contacting the major contributor is wrong, twice so if the maintainer is a relatively minor contributor to start with. Just because it recent came up elsewhere I'd like to recite the definition from Trond here again: http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-June/66.html By many of the creative roles enlisted there it's clear that Dave should be the maintainer. He's been the main contributor and chief architect for XFS for many year, while the maintainers came and went at the mercy of SGI. This is not meant to bad mouth either of you as I think you're doing a reasonably good job compared to other maintainers, but at the same time the direction is set by other people that have a much longer involvement with the project, and having them officially in control would help us forward a lot. It would also avoid having to spend considerable resources to train every new generation of SGI maintainer. Coming to and end I would like to maintain Dave Chinner as the primary XFS maintainer for all the work he has done as biggest contributor and architect of XFS since longer than I can remember, and I would love to retain Ben Myers as a co-maintainer for all the good work he has done maintaining and reviewing patches since November 2011. I think we're doing a decent job too. So thanks for that much at least. ;) I would also like to use this post as a public venue to condemn the unilateral smokey backroom decisions about XFS maintainership that SGI is trying to enforce on the community. That really didn't happen Christoph. It's not in my tree or in a pull request. Linus, let me know what you want to do. I do think we're doing a fair job over here, and (geez) I'm just trying to add Mark as my backup since Alex is too busy. I know the RH people want more control, and that's understandable, but they really don't need to replace me to get their code in. Ouch. Thanks, Ben Christoph is not a Red Hat person. Jeff is from Oracle. This is not a Red Hat vs SGI thing, Sorry if my read on that was wrong. Dave simply has earned the right to take on the formal leadership role of maintainer. Then we're gonna need some Reviewed-bys. ;) From: Ben Myers b...@sgi.com xfs: update maintainers Add Dave as maintainer of XFS. Signed-off-by: Ben Myers b...@sgi.com --- MAINTAINERS |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: b/MAINTAINERS === --- a/MAINTAINERS 2013-11-08 15:20:18.935186245 -0600 +++ b/MAINTAINERS 2013-11-08 15:22:50.685245977 -0600 @@ -9387,8 +9387,8 @@ F:drivers/xen/*swiotlb* XFS FILESYSTEM P: Silicon Graphics Inc +M: Dave Chinner dchin...@fromorbit.com M: Ben Myers b...@sgi.com -M: Alex Elder el...@kernel.org M: x...@oss.sgi.com L: x...@oss.sgi.com W: http://oss.sgi.com/projects/xfs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] update xfs maintainers
Hey Ric, On Fri, Nov 08, 2013 at 05:07:45PM -0500, Ric Wheeler wrote: On 11/08/2013 05:03 PM, Ben Myers wrote: Hey Ric, On Fri, Nov 08, 2013 at 03:50:21PM -0500, Ric Wheeler wrote: On 11/08/2013 03:46 PM, Ben Myers wrote: Hey Christoph, On Fri, Nov 08, 2013 at 11:34:24AM -0800, Christoph Hellwig wrote: On Fri, Nov 08, 2013 at 12:03:37PM -0600, Ben Myers wrote: Mark is replacing Alex as my backup because Alex is really busy at Linaro and asked to be taken off awhile ago. The holiday season is coming up and I fully intend to go off my meds, turn in to Fonzy the bear, and eat my hat. I need someone to watch the shop while I'm off exploring on Mars. I trust Mark to do that because he is totally awesome. Doing this as an unilateral decisions is not something that will win you a fan base. It's posted for review. While we never had anything reassembling a democracy in Linux Kernel development making decisions without even contacting the major contributor is wrong, twice so if the maintainer is a relatively minor contributor to start with. Just because it recent came up elsewhere I'd like to recite the definition from Trond here again: http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-June/66.html By many of the creative roles enlisted there it's clear that Dave should be the maintainer. He's been the main contributor and chief architect for XFS for many year, while the maintainers came and went at the mercy of SGI. This is not meant to bad mouth either of you as I think you're doing a reasonably good job compared to other maintainers, but at the same time the direction is set by other people that have a much longer involvement with the project, and having them officially in control would help us forward a lot. It would also avoid having to spend considerable resources to train every new generation of SGI maintainer. Coming to and end I would like to maintain Dave Chinner as the primary XFS maintainer for all the work he has done as biggest contributor and architect of XFS since longer than I can remember, and I would love to retain Ben Myers as a co-maintainer for all the good work he has done maintaining and reviewing patches since November 2011. I think we're doing a decent job too. So thanks for that much at least. ;) I would also like to use this post as a public venue to condemn the unilateral smokey backroom decisions about XFS maintainership that SGI is trying to enforce on the community. That really didn't happen Christoph. It's not in my tree or in a pull request. Linus, let me know what you want to do. I do think we're doing a fair job over here, and (geez) I'm just trying to add Mark as my backup since Alex is too busy. I know the RH people want more control, and that's understandable, but they really don't need to replace me to get their code in. Ouch. Thanks, Ben Christoph is not a Red Hat person. Jeff is from Oracle. This is not a Red Hat vs SGI thing, Sorry if my read on that was wrong. I do appreciate the work and effort you and the SGI team put in but think that this will be a good way to keep the community happier and even more productive going forward. Dave simply has earned the right to take on the formal leadership role of maintainer. Then we're gonna need some Reviewed-bys. ;) Those should come from the developers, thanks! I actually do need your Reviewed-by. We'll try and get this one in 3.13. ;) Thanks, Ben From: Ben Myers b...@sgi.com xfs: update maintainers Add Dave as maintainer of XFS. Signed-off-by: Ben Myers b...@sgi.com --- MAINTAINERS |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: b/MAINTAINERS === --- a/MAINTAINERS2013-11-08 15:20:18.935186245 -0600 +++ b/MAINTAINERS2013-11-08 15:22:50.685245977 -0600 @@ -9387,8 +9387,8 @@ F: drivers/xen/*swiotlb* XFS FILESYSTEM P: Silicon Graphics Inc +M: Dave Chinner dchin...@fromorbit.com M: Ben Myers b...@sgi.com -M: Alex Elder el...@kernel.org M: x...@oss.sgi.com L: x...@oss.sgi.com W: http://oss.sgi.com/projects/xfs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] xfs: simplify kmem_{zone_}zalloc
On Mon, Nov 04, 2013 at 06:21:05PM +0800, Gu Zheng wrote:
> Introduce flag KM_ZERO which is used to alloc zeroed entry, and convert
> kmem_{zone_}zalloc to call kmem_{zone_}alloc() with KM_ZERO directly,
> in order to avoid the setting to zero step.
> And following Dave's suggestion, make kmem_{zone_}zalloc static inline
> into kmem.h as they're now just a simple wrapper.
>
> V2:
> Make kmem_{zone_}zalloc static inline into kmem.h as Dave suggested.
>
> Signed-off-by: Gu Zheng
Applied. Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] xfs: simplify kmem_{zone_}zalloc
On Mon, Nov 04, 2013 at 06:21:05PM +0800, Gu Zheng wrote:
Introduce flag KM_ZERO which is used to alloc zeroed entry, and convert
kmem_{zone_}zalloc to call kmem_{zone_}alloc() with KM_ZERO directly,
in order to avoid the setting to zero step.
And following Dave's suggestion, make kmem_{zone_}zalloc static inline
into kmem.h as they're now just a simple wrapper.
V2:
Make kmem_{zone_}zalloc static inline into kmem.h as Dave suggested.
Signed-off-by: Gu Zheng guz.f...@cn.fujitsu.com
Applied. Thanks!
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] xfs:xfs_dir2_node.c: pointer use before check for null
On Fri, Oct 25, 2013 at 10:06:09AM -0500, Ben Myers wrote: > On Fri, Oct 25, 2013 at 03:53:25PM +0400, Denis Efremov wrote: > > ASSERT on args takes place after args dereference. > > This assertion is redundant since we are going to panic anyway. > > > > Found by Linux Driver Verification project (linuxtesting.org) - > > PVS-Studio analyzer. > > > > Signed-off-by: Denis Efremov > > Looks good. > > Reviewed-by: Ben Myers Applied. Thanks Denis. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xfs: fix possible NULL dereference
Hey Geyslan, On Wed, Oct 30, 2013 at 03:08:12PM -0500, Eric Sandeen wrote: > On 10/23/13 3:34 PM, Ben Myers wrote: > > > xfs: fix possible NULL dereference in xlog_verify_iclog > > > > In xlog_verify_iclog a debug check of the incore log buffers prints an > > error if icptr is null and then goes on to dereference the pointer > > regardless. Convert this to an assert so that the intention is clear. > > This was reported by Coverty. > > > > Reported-by: Geyslan G. Bem > > Signed-off-by: Ben Myers > > Reviewed-by: Eric Sandeen Applied this. Many thanks! ;) -Ben -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xfs: fix possible NULL dereference
Hey Geyslan, On Wed, Oct 30, 2013 at 03:08:12PM -0500, Eric Sandeen wrote: On 10/23/13 3:34 PM, Ben Myers wrote: xfs: fix possible NULL dereference in xlog_verify_iclog In xlog_verify_iclog a debug check of the incore log buffers prints an error if icptr is null and then goes on to dereference the pointer regardless. Convert this to an assert so that the intention is clear. This was reported by Coverty. Reported-by: Geyslan G. Bem geys...@gmail.com Signed-off-by: Ben Myers b...@sgi.com Reviewed-by: Eric Sandeen sand...@redhat.com Applied this. Many thanks! ;) -Ben -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] xfs:xfs_dir2_node.c: pointer use before check for null
On Fri, Oct 25, 2013 at 10:06:09AM -0500, Ben Myers wrote: On Fri, Oct 25, 2013 at 03:53:25PM +0400, Denis Efremov wrote: ASSERT on args takes place after args dereference. This assertion is redundant since we are going to panic anyway. Found by Linux Driver Verification project (linuxtesting.org) - PVS-Studio analyzer. Signed-off-by: Denis Efremov yefremov.de...@gmail.com Looks good. Reviewed-by: Ben Myers b...@sgi.com Applied. Thanks Denis. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] xfs:xfs_dir2_node.c: pointer use before check for null
On Fri, Oct 25, 2013 at 03:53:25PM +0400, Denis Efremov wrote: > ASSERT on args takes place after args dereference. > This assertion is redundant since we are going to panic anyway. > > Found by Linux Driver Verification project (linuxtesting.org) - > PVS-Studio analyzer. > > Signed-off-by: Denis Efremov Looks good. Reviewed-by: Ben Myers -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] xfs:xfs_dir2_node.c: pointer use before check for null
On Fri, Oct 25, 2013 at 03:53:25PM +0400, Denis Efremov wrote: ASSERT on args takes place after args dereference. This assertion is redundant since we are going to panic anyway. Found by Linux Driver Verification project (linuxtesting.org) - PVS-Studio analyzer. Signed-off-by: Denis Efremov yefremov.de...@gmail.com Looks good. Reviewed-by: Ben Myers b...@sgi.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xfs: fix possible NULL dereference
Hey Geyslan,
On Wed, Oct 23, 2013 at 08:58:51AM -0200, Geyslan Gregório Bem wrote:
> - Regarding the "possible new patch" subject, I humbly pass the ball to you.
>
> Thank you for the attention.
Thank you for the patch. I would really prefer to commit this showing
authorship from you, rather than a Reported-by. Can I mark you down?
Regards,
Ben
---
xfs: fix possible NULL dereference in xlog_verify_iclog
In xlog_verify_iclog a debug check of the incore log buffers prints an
error if icptr is null and then goes on to dereference the pointer
regardless. Convert this to an assert so that the intention is clear.
This was reported by Coverty.
Reported-by: Geyslan G. Bem
Signed-off-by: Ben Myers
---
fs/xfs/xfs_log.c |8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
Index: b/fs/xfs/xfs_log.c
===
--- a/fs/xfs/xfs_log.c 2013-10-23 14:52:47.875216875 -0500
+++ b/fs/xfs/xfs_log.c 2013-10-23 14:53:53.775245830 -0500
@@ -3714,11 +3714,9 @@ xlog_verify_iclog(
/* check validity of iclog pointers */
spin_lock(>l_icloglock);
icptr = log->l_iclog;
- for (i=0; i < log->l_iclog_bufs; i++) {
- if (icptr == NULL)
- xfs_emerg(log->l_mp, "%s: invalid ptr", __func__);
- icptr = icptr->ic_next;
- }
+ for (i=0; i < log->l_iclog_bufs; i++, icptr = icptr->ic_next)
+ ASSERT(icptr);
+
if (icptr != log->l_iclog)
xfs_emerg(log->l_mp, "%s: corrupt iclog ring", __func__);
spin_unlock(>l_icloglock);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xfs: fix possible NULL dereference
Hey Geyslan,
On Wed, Oct 23, 2013 at 08:58:51AM -0200, Geyslan Gregório Bem wrote:
- Regarding the possible new patch subject, I humbly pass the ball to you.
Thank you for the attention.
Thank you for the patch. I would really prefer to commit this showing
authorship from you, rather than a Reported-by. Can I mark you down?
Regards,
Ben
---
xfs: fix possible NULL dereference in xlog_verify_iclog
In xlog_verify_iclog a debug check of the incore log buffers prints an
error if icptr is null and then goes on to dereference the pointer
regardless. Convert this to an assert so that the intention is clear.
This was reported by Coverty.
Reported-by: Geyslan G. Bem geys...@gmail.com
Signed-off-by: Ben Myers b...@sgi.com
---
fs/xfs/xfs_log.c |8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
Index: b/fs/xfs/xfs_log.c
===
--- a/fs/xfs/xfs_log.c 2013-10-23 14:52:47.875216875 -0500
+++ b/fs/xfs/xfs_log.c 2013-10-23 14:53:53.775245830 -0500
@@ -3714,11 +3714,9 @@ xlog_verify_iclog(
/* check validity of iclog pointers */
spin_lock(log-l_icloglock);
icptr = log-l_iclog;
- for (i=0; i log-l_iclog_bufs; i++) {
- if (icptr == NULL)
- xfs_emerg(log-l_mp, %s: invalid ptr, __func__);
- icptr = icptr-ic_next;
- }
+ for (i=0; i log-l_iclog_bufs; i++, icptr = icptr-ic_next)
+ ASSERT(icptr);
+
if (icptr != log-l_iclog)
xfs_emerg(log-l_mp, %s: corrupt iclog ring, __func__);
spin_unlock(log-l_icloglock);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xfs: fix possible NULL dereference
Hey Gents,
On Wed, Oct 23, 2013 at 09:02:54AM +1100, Dave Chinner wrote:
> On Tue, Oct 22, 2013 at 04:19:44PM -0500, Eric Sandeen wrote:
> > On 10/22/13 4:03 PM, Dave Chinner wrote:
> > > On Tue, Oct 22, 2013 at 03:49:01PM -0500, Eric Sandeen wrote:
> > >> On 10/22/13 3:39 PM, Dave Chinner wrote:
> > >>> On Tue, Oct 22, 2013 at 08:12:51AM -0200, Geyslan Gregório Bem wrote:
> > >>>> 2013/10/21 Dave Chinner :
> > >>>>> On Mon, Oct 21, 2013 at 07:00:59PM -0500, Eric Sandeen wrote:
> > >>>>>> On 10/21/13 6:56 PM, Dave Chinner wrote:
> > >>>>>>> On Mon, Oct 21, 2013 at 06:18:49PM -0500, Ben Myers wrote:
> > >>>>>
> > >>>>> Yes, but to continue the Devil's Advocate argument, the purpose of
> > >>>>> debug code isn't to enlightent the casual reader or drive-by
> > >>>>> patchers - it's to make life easier for people who actually spend
> > >>>>> time debugging the code. And the people who need the debug code
> > >>>>> are expected to understand why an ASSERT is not necessary. :)
> > >>>>>
> > >>>> Dave, Eric and Ben,
> > >>>>
> > >>>> This was catched by coverity (CID 102348).
> > >>>
> > >>> You should have put that in the patch description.
> > >>>
> > >>> Now I understand why there's been a sudden surge of irrelevant one
> > >>> line changes from random people that have never touched XFS before.
> > >>>
> > >>>
> > >>>
> > >>> Ok, lets churn the code just to shut the stupid checker up. This
> > >>> doesn't fix a bug, it doesn't change behaviour, it just makes
> > >>> coverity happy. Convert it to the for loop plus ASSERT I mentioned
> > >>> in a previous message.
> > >>
> > >> You know, I respectfully disagree, but we might just have to agree
> > >> to disagree. The code, as it stands, tests for a null ptr
> > >> and then dereferences it. That's always going to raise some
> > >> eyebrows, coverity or not, debug code or not, drive by or not.
> > >
> > >> So even for future developers, making the code more self-
> > >> documenting about this behavior would be a plus, whether it's by
> > >> comment, by explicit ASSERT(), or whatever. (I don't think
> > >> that xfs_emerg() has quite enough context to make it obvious.)
> > >
> > > Sure, but if weren't for the fact that Coverity warned about it,
> > > nobody other that us people who work on the XFS code day in, day out
> > > would have even cared about it.
> > >
> > > That's kind of my point - again, as the Devil's Advocate - that
> > > coverity is encouraging drive-by "fixes" by people who don't
> > > actually understand any of the context, history and/or culture
> > > surrounding the code being modified.
> >
> > They shouldn't have to, the code (or comments therein) should
> > make it obvious. ;) (in a perfect world...)
>
> Obvious to whom, exactly?
>
> That's the point I'm trying to make - "#ifdef DEBUG", two
> comments indicating that it's validating the list and printing a
> message just before it goes boom. That's pretty obvious code to
> anyone who is used to tracking down corrupted list problems...
>
> > > I have no problems with real bugs being fixed, but if we are
> > > modifying code for no gain other than closing "coverity doesn't like
> > > it" bugs, then we *should* be questioning whether the change is
> > > really necessary.
> >
> > But let's give Geyslan the benefit of the doubt, and realize that
> > Coverity does find real things, and even if it originated w/ a
> > Coverity CID, when one sees:
> >
> > if (!a)
> > printk("a thing\n")
> >
> > a = a->b = . . .
> >
> > it looks suspicious to pretty much anyone. I don't think Geyslan
> > sent it to shut Coverity up, he sent it because it looked like
> > a bug worth fixing (after Coverity spotted it).
> >
> > Let's not be too hard on him for trying; I appreciate it more
> > than spelling fixes and whitespace cleanups. ;)
>
> True, point taken.
So, uh, lets go with the ASSERT approach then? It seems to be a reasonable
middle ground. ;)
Regards,
Ben
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xfs: fix possible NULL dereference
Hey Gents, On Wed, Oct 23, 2013 at 09:02:54AM +1100, Dave Chinner wrote: On Tue, Oct 22, 2013 at 04:19:44PM -0500, Eric Sandeen wrote: On 10/22/13 4:03 PM, Dave Chinner wrote: On Tue, Oct 22, 2013 at 03:49:01PM -0500, Eric Sandeen wrote: On 10/22/13 3:39 PM, Dave Chinner wrote: On Tue, Oct 22, 2013 at 08:12:51AM -0200, Geyslan Gregório Bem wrote: 2013/10/21 Dave Chinner da...@fromorbit.com: On Mon, Oct 21, 2013 at 07:00:59PM -0500, Eric Sandeen wrote: On 10/21/13 6:56 PM, Dave Chinner wrote: On Mon, Oct 21, 2013 at 06:18:49PM -0500, Ben Myers wrote: Yes, but to continue the Devil's Advocate argument, the purpose of debug code isn't to enlightent the casual reader or drive-by patchers - it's to make life easier for people who actually spend time debugging the code. And the people who need the debug code are expected to understand why an ASSERT is not necessary. :) Dave, Eric and Ben, This was catched by coverity (CID 102348). You should have put that in the patch description. Now I understand why there's been a sudden surge of irrelevant one line changes from random people that have never touched XFS before. sigh Ok, lets churn the code just to shut the stupid checker up. This doesn't fix a bug, it doesn't change behaviour, it just makes coverity happy. Convert it to the for loop plus ASSERT I mentioned in a previous message. You know, I respectfully disagree, but we might just have to agree to disagree. The code, as it stands, tests for a null ptr and then dereferences it. That's always going to raise some eyebrows, coverity or not, debug code or not, drive by or not. So even for future developers, making the code more self- documenting about this behavior would be a plus, whether it's by comment, by explicit ASSERT(), or whatever. (I don't think that xfs_emerg() has quite enough context to make it obvious.) Sure, but if weren't for the fact that Coverity warned about it, nobody other that us people who work on the XFS code day in, day out would have even cared about it. That's kind of my point - again, as the Devil's Advocate - that coverity is encouraging drive-by fixes by people who don't actually understand any of the context, history and/or culture surrounding the code being modified. They shouldn't have to, the code (or comments therein) should make it obvious. ;) (in a perfect world...) Obvious to whom, exactly? That's the point I'm trying to make - #ifdef DEBUG, two comments indicating that it's validating the list and printing a message just before it goes boom. That's pretty obvious code to anyone who is used to tracking down corrupted list problems... I have no problems with real bugs being fixed, but if we are modifying code for no gain other than closing coverity doesn't like it bugs, then we *should* be questioning whether the change is really necessary. But let's give Geyslan the benefit of the doubt, and realize that Coverity does find real things, and even if it originated w/ a Coverity CID, when one sees: if (!a) printk(a thing\n) a = a-b = . . . it looks suspicious to pretty much anyone. I don't think Geyslan sent it to shut Coverity up, he sent it because it looked like a bug worth fixing (after Coverity spotted it). Let's not be too hard on him for trying; I appreciate it more than spelling fixes and whitespace cleanups. ;) True, point taken. So, uh, lets go with the ASSERT approach then? It seems to be a reasonable middle ground. ;) Regards, Ben -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xfs: fix possible NULL dereference
Hey, On Mon, Oct 21, 2013 at 06:12:18PM -0500, Eric Sandeen wrote: > On 10/21/13 5:44 PM, Dave Chinner wrote: > > On Mon, Oct 21, 2013 at 03:58:23PM -0500, Eric Sandeen wrote: > >> On 10/21/13 1:32 PM, Geyslan G. Bem wrote: > >>> This patch puts a 'break' in the true branch, avoiding the > >>> 'icptr->ic_next' > >>> dereferencing. > >> > >> Reviewed-by: Eric Sandeen > > > > Actually, NACK. > > I felt that one coming ;) > > >> Hm, yeah - cmn_err(CE_PANIC, " " ); used to BUG_ON, but the newer > >> xfs_emerg() doesn't. > >> > >> Dave, was that intentional? > > > > Of course it was. ;) xfs_emerg() is only called from the debug code > > in xlog_verify_iclog(), xlog_verify_tail_lsn and assfail(). > > > > In the case of assfail(), it has it's own BUG() call, so it does > > everything just fine. > > > > In the case of xlog_verify_iclog() when icptr is NULL, it will > > panic immediately after the message is printed, just like the old > > code. i.e. this patch isn't fixing anything we need fixed. > > A BUG() is probably warranted, then. I tend to agree with Eric on this point. If we want to crash, I'd rather our intent be very clear, rather than just see a null ptr deref. ;) Regards, Ben -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xfs: fix possible NULL dereference
Hey, On Mon, Oct 21, 2013 at 06:12:18PM -0500, Eric Sandeen wrote: On 10/21/13 5:44 PM, Dave Chinner wrote: On Mon, Oct 21, 2013 at 03:58:23PM -0500, Eric Sandeen wrote: On 10/21/13 1:32 PM, Geyslan G. Bem wrote: This patch puts a 'break' in the true branch, avoiding the 'icptr-ic_next' dereferencing. Reviewed-by: Eric Sandeen sand...@redhat.com Actually, NACK. I felt that one coming ;) Hm, yeah - cmn_err(CE_PANIC, ); used to BUG_ON, but the newer xfs_emerg() doesn't. Dave, was that intentional? Of course it was. ;) xfs_emerg() is only called from the debug code in xlog_verify_iclog(), xlog_verify_tail_lsn and assfail(). In the case of assfail(), it has it's own BUG() call, so it does everything just fine. In the case of xlog_verify_iclog() when icptr is NULL, it will panic immediately after the message is printed, just like the old code. i.e. this patch isn't fixing anything we need fixed. A BUG() is probably warranted, then. I tend to agree with Eric on this point. If we want to crash, I'd rather our intent be very clear, rather than just see a null ptr deref. ;) Regards, Ben -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] XFS fixes for 3.12-rc4
Hi Linus, Please pull in these bugfixes for xfs. There are lockdep annotations for project quotas, a fix for dirent dtype support on v4 filesystems, a fix for a memory leak in recovery, and a fix for the build error that resulted from it. D'oh. Thanks, Ben The following changes since commit 997def25e4b9cee3b01609e18a52f926bca8bd2b: xfs: fix node forward in xfs_node_toosmall (2013-09-26 10:38:17 -0500) are available in the git repository at: git://oss.sgi.com/xfs/xfs.git tags/xfs-for-linus-v3.12-rc4 for you to fetch changes up to b2a42f78ab475f4730300b0e9568bc3b2587d112: xfs: Use kmem_free() instead of free() (2013-10-04 13:56:12 -0500) xfs: bugfixes for 3.12-rc4 - lockdep fix for project quotas - fix for dirent dtype support on v4 filesystems - fix for a memory leak in recovery - fix for build failure due to the recovery fix Dave Chinner (2): xfs: lockdep needs to know about 3 dquot-deep nesting xfs: dirent dtype presence is dependent on directory magic numbers Thierry Reding (1): xfs: Use kmem_free() instead of free() tingu...@sgi.com (1): xfs: fix memory leak in xlog_recover_add_to_trans fs/xfs/xfs_dir2_block.c | 6 +++--- fs/xfs/xfs_dir2_format.h | 51 +++ fs/xfs/xfs_dir2_readdir.c | 4 ++-- fs/xfs/xfs_dir2_sf.c | 6 +++--- fs/xfs/xfs_dquot.c| 19 +++--- fs/xfs/xfs_log_recover.c | 1 + 6 files changed, 45 insertions(+), 42 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] XFS fixes for 3.12-rc4
Hi Linus, Please pull in these bugfixes for xfs. There are lockdep annotations for project quotas, a fix for dirent dtype support on v4 filesystems, a fix for a memory leak in recovery, and a fix for the build error that resulted from it. D'oh. Thanks, Ben The following changes since commit 997def25e4b9cee3b01609e18a52f926bca8bd2b: xfs: fix node forward in xfs_node_toosmall (2013-09-26 10:38:17 -0500) are available in the git repository at: git://oss.sgi.com/xfs/xfs.git tags/xfs-for-linus-v3.12-rc4 for you to fetch changes up to b2a42f78ab475f4730300b0e9568bc3b2587d112: xfs: Use kmem_free() instead of free() (2013-10-04 13:56:12 -0500) xfs: bugfixes for 3.12-rc4 - lockdep fix for project quotas - fix for dirent dtype support on v4 filesystems - fix for a memory leak in recovery - fix for build failure due to the recovery fix Dave Chinner (2): xfs: lockdep needs to know about 3 dquot-deep nesting xfs: dirent dtype presence is dependent on directory magic numbers Thierry Reding (1): xfs: Use kmem_free() instead of free() tingu...@sgi.com (1): xfs: fix memory leak in xlog_recover_add_to_trans fs/xfs/xfs_dir2_block.c | 6 +++--- fs/xfs/xfs_dir2_format.h | 51 +++ fs/xfs/xfs_dir2_readdir.c | 4 ++-- fs/xfs/xfs_dir2_sf.c | 6 +++--- fs/xfs/xfs_dquot.c| 19 +++--- fs/xfs/xfs_log_recover.c | 1 + 6 files changed, 45 insertions(+), 42 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xfs: Use kmem_free() instead of free()
On Tue, Oct 01, 2013 at 10:00:43AM -0500, Mark Tinguely wrote: > On 10/01/13 09:47, Thierry Reding wrote: > >This fixes a build failure caused by calling the free() function which > >does not exist in the Linux kernel. > > > >Signed-off-by: Thierry Reding > >--- > > fs/xfs/xfs_log_recover.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > >diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c > >index 4324058..3979749 100644 > >--- a/fs/xfs/xfs_log_recover.c > >+++ b/fs/xfs/xfs_log_recover.c > >@@ -1585,7 +1585,7 @@ xlog_recover_add_to_trans( > > "bad number of regions (%d) in inode log format", > > in_f->ilf_size); > > ASSERT(0); > >-free(ptr); > >+kmem_free(ptr); > > return XFS_ERROR(EIO); > > } > > > > Looks good. I will remove the other list items in another patch. > > Reviewed-by: Mark Tinguely Gah. Build Fail. Apparently things were getting a little punchy over here. Applied, and thanks Thierry. -Ben -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xfs: Use kmem_free() instead of free()
On Tue, Oct 01, 2013 at 10:00:43AM -0500, Mark Tinguely wrote: On 10/01/13 09:47, Thierry Reding wrote: This fixes a build failure caused by calling the free() function which does not exist in the Linux kernel. Signed-off-by: Thierry Redingtred...@nvidia.com --- fs/xfs/xfs_log_recover.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index 4324058..3979749 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -1585,7 +1585,7 @@ xlog_recover_add_to_trans( bad number of regions (%d) in inode log format, in_f-ilf_size); ASSERT(0); -free(ptr); +kmem_free(ptr); return XFS_ERROR(EIO); } Looks good. I will remove the other list items in another patch. Reviewed-by: Mark Tinguely tingu...@sgi.com Gah. Build Fail. Apparently things were getting a little punchy over here. Applied, and thanks Thierry. -Ben -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] XFS fixes for 3.12-rc3
Hi Linus, Please pull these bugfixes for xfs. There is a fix for an assert caused by a spurious directory block collapse, a fix for recovery of a block over stale metadata from a previous mkfs, a cleanup for the eofblocks ioctl, and fixes for locking issues in xfs_inode_free and log item removal from the active item list. Thanks, Ben The following changes since commit 272b98c6455f00884f0350f775c5342358ebb73f: Linux 3.12-rc1 (2013-09-16 16:17:51 -0400) are available in the git repository at: git://oss.sgi.com/xfs/xfs tags/xfs-for-linus-v3.12-rc3 for you to fetch changes up to 997def25e4b9cee3b01609e18a52f926bca8bd2b: xfs: fix node forward in xfs_node_toosmall (2013-09-26 10:38:17 -0500) xfs: bugfixes for 3.12-rc3 - fix for directory node collapse regression - fix for recovery over stale on disk structures - fix for eofblocks ioctl - fix asserts in xfs_inode_free - lock the ail before removing an item from it Dave Chinner (4): xfs: lock the AIL before removing the buffer item xfs: asserting lock not held during freeing not valid xfs: fix XFS_IOC_FREE_EOFBLOCKS definition xfs: log recovery lsn ordering needs uuid check Mark Tinguely (1): xfs: fix node forward in xfs_node_toosmall fs/xfs/xfs_buf_item.c| 1 + fs/xfs/xfs_da_btree.c| 5 ++-- fs/xfs/xfs_fs.h | 2 +- fs/xfs/xfs_icache.c | 9 +++--- fs/xfs/xfs_log_recover.c | 73 ++-- 5 files changed, 68 insertions(+), 22 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] XFS fixes for 3.12-rc3
Hi Linus, Please pull these bugfixes for xfs. There is a fix for an assert caused by a spurious directory block collapse, a fix for recovery of a block over stale metadata from a previous mkfs, a cleanup for the eofblocks ioctl, and fixes for locking issues in xfs_inode_free and log item removal from the active item list. Thanks, Ben The following changes since commit 272b98c6455f00884f0350f775c5342358ebb73f: Linux 3.12-rc1 (2013-09-16 16:17:51 -0400) are available in the git repository at: git://oss.sgi.com/xfs/xfs tags/xfs-for-linus-v3.12-rc3 for you to fetch changes up to 997def25e4b9cee3b01609e18a52f926bca8bd2b: xfs: fix node forward in xfs_node_toosmall (2013-09-26 10:38:17 -0500) xfs: bugfixes for 3.12-rc3 - fix for directory node collapse regression - fix for recovery over stale on disk structures - fix for eofblocks ioctl - fix asserts in xfs_inode_free - lock the ail before removing an item from it Dave Chinner (4): xfs: lock the AIL before removing the buffer item xfs: asserting lock not held during freeing not valid xfs: fix XFS_IOC_FREE_EOFBLOCKS definition xfs: log recovery lsn ordering needs uuid check Mark Tinguely (1): xfs: fix node forward in xfs_node_toosmall fs/xfs/xfs_buf_item.c| 1 + fs/xfs/xfs_da_btree.c| 5 ++-- fs/xfs/xfs_fs.h | 2 +- fs/xfs/xfs_icache.c | 9 +++--- fs/xfs/xfs_log_recover.c | 73 ++-- 5 files changed, 68 insertions(+), 22 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [XFS MAINTAINERS] fs/xfs/xfs_dir2_node.c: xfs: xfs_dir2_leafn_add: Variables Uninitialized
Hi Geyslan,
On Fri, Sep 27, 2013 at 02:59:12PM -0300, Geyslan Gregório Bem wrote:
> Hi Maintainers,
>
> I suppose the variables "highstale" and "lowstale" are being used despite
> not having been initialized.
>
> File: fs/xfs/xfs_dir2_node.c
> Function: xfs_dir2_leafn_add
>
> L491:
> > /*
> > * Insert the new entry, log everything.
> > */
> > lep = xfs_dir3_leaf_find_entry(, ents, index, compact, lowstale,
> > highstale, , );
>
> The only place they are started up is within this condition:
>
> L480:
> > if (compact)
> > xfs_dir3_leaf_compact_x1(, ents, , ,
> > , , );
>
> So, if it is not compact, both have garbage.
Thanks for the report. That sounds pretty bad. Lets see...
421 static int /* error */
422 xfs_dir2_leafn_add(
423 struct xfs_buf *bp,/* leaf buffer */
424 xfs_da_args_t *args, /* operation arguments */
425 int index) /* insertion pt for new
entry */
426 {
...
476 /*
477 * Compact out all but one stale leaf entry. Leaves behind
478 * the entry closest to index.
479 */
480 if (compact)
481 xfs_dir3_leaf_compact_x1(, ents, , ,
482 , , );
483 else if (leafhdr.stale) {
484 /*
485 * Set impossible logging indices for this case.
486 */
487 lfloglow = leafhdr.count;
488 lfloghigh = -1;
489 }
490
491 /*
492 * Insert the new entry, log everything.
493 */
494 lep = xfs_dir3_leaf_find_entry(, ents, index, compact,
lowstale,
495highstale, , );
If compact is set at 481 we pass the addresses of highstale and lowstale to
xfs_dir3_leaf_compact_x1, which passes them to xfs_dir3_leaf_find_stale, which
makes assignments to both variables unconditionally. Later at 494 we pass
compact, lowstale, and highstale to xfs_dir3_leaf_find_entry.
So we're ok if compact is set...
555 struct xfs_dir2_leaf_entry *
556 xfs_dir3_leaf_find_entry(
557 struct xfs_dir3_icleaf_hdr *leafhdr,
558 struct xfs_dir2_leaf_entry *ents,
559 int index, /* leaf table position */
560 int compact,/* need to compact leaves */
561 int lowstale, /* index of prev stale leaf
*/
562 int highstale, /* index of next stale leaf
*/
563 int *lfloglow, /* low leaf logging index */
564 int *lfloghigh) /* high leaf logging index
*/
565 {
...
587 /*
588 * There are stale entries.
589 *
590 * We will use one of them for the new entry. It's probably not at
591 * the right location, so we'll have to shift some up or down first.
592 *
593 * If we didn't compact before, we need to find the nearest stale
594 * entries before and after our insertion point.
595 */
596 if (compact == 0)
597 xfs_dir3_leaf_find_stale(leafhdr, ents, index,
598 , );
In xfs_dir3_leaf_find_entry, it looks like if compact is not set, we will pass
the addresses of lowstale and highstale to xfs_dir3_leaf_find_stale which
appears to make assignments to them unconditionally. It looks like
xfs_dir3_leaf_find_entry doesn't read from lowstale and highstale until after
598. I think this should take care of the !compact case too. Do you agree?
Thanks much,
Ben
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [XFS MAINTAINERS] fs/xfs/xfs_dir2_node.c: xfs: xfs_dir2_leafn_add: Variables Uninitialized
Hi Geyslan,
On Fri, Sep 27, 2013 at 02:59:12PM -0300, Geyslan Gregório Bem wrote:
Hi Maintainers,
I suppose the variables highstale and lowstale are being used despite
not having been initialized.
File: fs/xfs/xfs_dir2_node.c
Function: xfs_dir2_leafn_add
L491:
/*
* Insert the new entry, log everything.
*/
lep = xfs_dir3_leaf_find_entry(leafhdr, ents, index, compact, lowstale,
highstale, lfloglow, lfloghigh);
The only place they are started up is within this condition:
L480:
if (compact)
xfs_dir3_leaf_compact_x1(leafhdr, ents, index, lowstale,
highstale, lfloglow, lfloghigh);
So, if it is not compact, both have garbage.
Thanks for the report. That sounds pretty bad. Lets see...
421 static int /* error */
422 xfs_dir2_leafn_add(
423 struct xfs_buf *bp,/* leaf buffer */
424 xfs_da_args_t *args, /* operation arguments */
425 int index) /* insertion pt for new
entry */
426 {
...
476 /*
477 * Compact out all but one stale leaf entry. Leaves behind
478 * the entry closest to index.
479 */
480 if (compact)
481 xfs_dir3_leaf_compact_x1(leafhdr, ents, index, lowstale,
482 highstale, lfloglow, lfloghigh);
483 else if (leafhdr.stale) {
484 /*
485 * Set impossible logging indices for this case.
486 */
487 lfloglow = leafhdr.count;
488 lfloghigh = -1;
489 }
490
491 /*
492 * Insert the new entry, log everything.
493 */
494 lep = xfs_dir3_leaf_find_entry(leafhdr, ents, index, compact,
lowstale,
495highstale, lfloglow, lfloghigh);
If compact is set at 481 we pass the addresses of highstale and lowstale to
xfs_dir3_leaf_compact_x1, which passes them to xfs_dir3_leaf_find_stale, which
makes assignments to both variables unconditionally. Later at 494 we pass
compact, lowstale, and highstale to xfs_dir3_leaf_find_entry.
So we're ok if compact is set...
555 struct xfs_dir2_leaf_entry *
556 xfs_dir3_leaf_find_entry(
557 struct xfs_dir3_icleaf_hdr *leafhdr,
558 struct xfs_dir2_leaf_entry *ents,
559 int index, /* leaf table position */
560 int compact,/* need to compact leaves */
561 int lowstale, /* index of prev stale leaf
*/
562 int highstale, /* index of next stale leaf
*/
563 int *lfloglow, /* low leaf logging index */
564 int *lfloghigh) /* high leaf logging index
*/
565 {
...
587 /*
588 * There are stale entries.
589 *
590 * We will use one of them for the new entry. It's probably not at
591 * the right location, so we'll have to shift some up or down first.
592 *
593 * If we didn't compact before, we need to find the nearest stale
594 * entries before and after our insertion point.
595 */
596 if (compact == 0)
597 xfs_dir3_leaf_find_stale(leafhdr, ents, index,
598 lowstale, highstale);
In xfs_dir3_leaf_find_entry, it looks like if compact is not set, we will pass
the addresses of lowstale and highstale to xfs_dir3_leaf_find_stale which
appears to make assignments to them unconditionally. It looks like
xfs_dir3_leaf_find_entry doesn't read from lowstale and highstale until after
598. I think this should take care of the !compact case too. Do you agree?
Thanks much,
Ben
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] XFS updates #2 for 3.12-rc1
Hi Linus, Please pull these XFS updates for 3.12-rc1. There is a feature to support defrag on CRC enabled filesystems, some bugfixes, and cleanups. Thanks, Ben The following changes since commit 1d03c6fa88af35e55047a1f2ab116f0fdf2f55aa: xfs: XFS_MOUNT_QUOTA_ALL needed by userspace (2013-09-03 15:00:06 -0500) are available in the git repository at: git://oss.sgi.com/xfs/xfs tags/xfs-for-linus-v3.12-rc1-2 for you to fetch changes up to 08474ed639e971e9d5a877cf7aba7ef91d847ae9: xfs: remove dead code from xlog_recover_inode_pass2 (2013-09-12 09:51:49 -0500) xfs: update #2 for v3.12-rc1 Here we have defrag support for v5 superblock, a number of bugfixes and a cleanup or two. - defrag support for CRC filesystems - fix endian worning in xlog_recover_get_buf_lsn - fixes for sparse warnings - fix for assert in xfs_dir3_leaf_hdr_from_disk - fix for log recovery of remote symlinks - fix for log recovery of btree root splits - fixes formemory allocation failures with ACLs - fix for assert in xfs_buf_item_relse - fix for assert in xfs_inode_buf_verify - fix an assignment in an assert that should be a test in xfs_bmbt_change_owner - remove dead code in xlog_recover_inode_pass2 Dan Carpenter (1): xfs: = vs == typo in ASSERT() Dave Chinner (11): xfs: fix endian warning in xlog_recover_get_buf_lsn() xfs: fix some minor sparse warnings xfs: check magic numbers in dir3 leaf verifier first xfs: swap extents operations for CRC filesystems xfs: recovery of swap extents operations for CRC filesystems xfs: set remote symlink buffer type for recovery xfs: ensure we copy buffer type in da btree root splits xfs: fix memory allocation failures with ACLs xfs: factor all the kmalloc-or-vmalloc fallback allocations xfs: aborted buf items can be in the AIL. xfs: don't assert fail on bad inode numbers Mark Tinguely (1): xfs: remove dead code from xlog_recover_inode_pass2 fs/xfs/kmem.c| 15 - fs/xfs/kmem.h| 9 +-- fs/xfs/xfs_acl.c | 12 ++-- fs/xfs/xfs_bmap.c| 2 +- fs/xfs/xfs_bmap_btree.c | 44 fs/xfs/xfs_bmap_btree.h | 4 ++ fs/xfs/xfs_bmap_util.c | 69 --- fs/xfs/xfs_btree.c | 170 ++- fs/xfs/xfs_btree.h | 19 -- fs/xfs/xfs_buf_item.c| 24 +-- fs/xfs/xfs_da_btree.c| 1 + fs/xfs/xfs_dir2_leaf.c | 20 -- fs/xfs/xfs_dquot_item.c | 3 +- fs/xfs/xfs_extent_busy.c | 3 +- fs/xfs/xfs_icache.c | 4 +- fs/xfs/xfs_icache.h | 4 ++ fs/xfs/xfs_inode_buf.c | 10 ++- fs/xfs/xfs_inode_buf.h | 18 ++--- fs/xfs/xfs_ioctl.c | 36 -- fs/xfs/xfs_ioctl32.c | 18 ++--- fs/xfs/xfs_itable.c | 7 +- fs/xfs/xfs_log.c | 3 +- fs/xfs/xfs_log_format.h | 8 ++- fs/xfs/xfs_log_recover.c | 122 +++--- fs/xfs/xfs_symlink.c | 2 + 25 files changed, 461 insertions(+), 166 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] XFS updates #2 for 3.12-rc1
Hi Linus, Please pull these XFS updates for 3.12-rc1. There is a feature to support defrag on CRC enabled filesystems, some bugfixes, and cleanups. Thanks, Ben The following changes since commit 1d03c6fa88af35e55047a1f2ab116f0fdf2f55aa: xfs: XFS_MOUNT_QUOTA_ALL needed by userspace (2013-09-03 15:00:06 -0500) are available in the git repository at: git://oss.sgi.com/xfs/xfs tags/xfs-for-linus-v3.12-rc1-2 for you to fetch changes up to 08474ed639e971e9d5a877cf7aba7ef91d847ae9: xfs: remove dead code from xlog_recover_inode_pass2 (2013-09-12 09:51:49 -0500) xfs: update #2 for v3.12-rc1 Here we have defrag support for v5 superblock, a number of bugfixes and a cleanup or two. - defrag support for CRC filesystems - fix endian worning in xlog_recover_get_buf_lsn - fixes for sparse warnings - fix for assert in xfs_dir3_leaf_hdr_from_disk - fix for log recovery of remote symlinks - fix for log recovery of btree root splits - fixes formemory allocation failures with ACLs - fix for assert in xfs_buf_item_relse - fix for assert in xfs_inode_buf_verify - fix an assignment in an assert that should be a test in xfs_bmbt_change_owner - remove dead code in xlog_recover_inode_pass2 Dan Carpenter (1): xfs: = vs == typo in ASSERT() Dave Chinner (11): xfs: fix endian warning in xlog_recover_get_buf_lsn() xfs: fix some minor sparse warnings xfs: check magic numbers in dir3 leaf verifier first xfs: swap extents operations for CRC filesystems xfs: recovery of swap extents operations for CRC filesystems xfs: set remote symlink buffer type for recovery xfs: ensure we copy buffer type in da btree root splits xfs: fix memory allocation failures with ACLs xfs: factor all the kmalloc-or-vmalloc fallback allocations xfs: aborted buf items can be in the AIL. xfs: don't assert fail on bad inode numbers Mark Tinguely (1): xfs: remove dead code from xlog_recover_inode_pass2 fs/xfs/kmem.c| 15 - fs/xfs/kmem.h| 9 +-- fs/xfs/xfs_acl.c | 12 ++-- fs/xfs/xfs_bmap.c| 2 +- fs/xfs/xfs_bmap_btree.c | 44 fs/xfs/xfs_bmap_btree.h | 4 ++ fs/xfs/xfs_bmap_util.c | 69 --- fs/xfs/xfs_btree.c | 170 ++- fs/xfs/xfs_btree.h | 19 -- fs/xfs/xfs_buf_item.c| 24 +-- fs/xfs/xfs_da_btree.c| 1 + fs/xfs/xfs_dir2_leaf.c | 20 -- fs/xfs/xfs_dquot_item.c | 3 +- fs/xfs/xfs_extent_busy.c | 3 +- fs/xfs/xfs_icache.c | 4 +- fs/xfs/xfs_icache.h | 4 ++ fs/xfs/xfs_inode_buf.c | 10 ++- fs/xfs/xfs_inode_buf.h | 18 ++--- fs/xfs/xfs_ioctl.c | 36 -- fs/xfs/xfs_ioctl32.c | 18 ++--- fs/xfs/xfs_itable.c | 7 +- fs/xfs/xfs_log.c | 3 +- fs/xfs/xfs_log_format.h | 8 ++- fs/xfs/xfs_log_recover.c | 122 +++--- fs/xfs/xfs_symlink.c | 2 + 25 files changed, 461 insertions(+), 166 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] XFS updates for 3.12-rc1
Hi Linus, Please pull these XFS updates for 3.12-rc1. Most of this is related to the libxfs kernel/userspace sync, there is also project quota work, performance work in the log, recovery, and the CIL. User namespace support has been added, directory entries now have file type support, there is work to cleanup log space reservations, a bunch of spelling cleanups, and bug fixes. Thanks, Ben The following changes since commit ad81f0545ef01ea651886dddac4bef6cec930092: Linux 3.11-rc1 (2013-07-14 15:18:27 -0700) are available in the git repository at: git://oss.sgi.com/xfs/xfs tags/xfs-for-linus-v3.12-rc1 for you to fetch changes up to 1d03c6fa88af35e55047a1f2ab116f0fdf2f55aa: xfs: XFS_MOUNT_QUOTA_ALL needed by userspace (2013-09-03 15:00:06 -0500) xfs: update for v3.12-rc1 For 3.12-rc1 there are a number of bugfixes in addition to work to ease usage of shared code between libxfs and the kernel, the rest of the work to enable project and group quotas to be used simultaneously, performance optimisations in the log and the CIL, directory entry file type support, fixes for log space reservations, some spelling/grammar cleanups, and the addition of user namespace support. - introduce readahead to log recovery - add directory entry file type support - fix a number of spelling errors in comments - introduce new Q_XGETQSTATV quotactl for project quotas - add USER_NS support - log space reservation rework - CIL optimisations - kernel/userspace libxfs rework Brian Foster (1): xfs: check correct status variable for xfs_inobt_get_rec() call Chandra Seetharaman (5): xfs: Fix a deadlock in xfs_log_commit_cil() code path xfs: Initialize all quota inodes to be NULLFSINO xfs: Start using pquotaino from the superblock. quota: Add a new quotactl command Q_XGETQSTATV xfs: Add support for the Q_XGETQSTATV Dan Carpenter (1): xfs: check for underflow in xfs_iformat_fork() Dave Chinner (53): xfs: di_flushiter considered harmful xfs: separate out log format definitions xfs: split out inode log item format definition xfs: split out buf log item format definitions xfs: split out EFI/EFD log item format definition xfs: separate dquot on disk format definitions out of xfs_quota.h xfs: separate icreate log format definitions from xfs_icreate_item.h xfs: split out on-disk transaction definitions xfs: introduce xfs_rtalloc_defs.h xfs: introduce xfs_quota_defs.h xfs: sync minor header differences needed by userspace. xfs: split out transaction reservation code xfs: move inode fork definitions to a new header file xfs: move unrelated definitions out of xfs_inode.h xfs: introduce xfs_inode_buf.c for inode buffer operations xfs: move getdents code into it's own file xfs: reshuffle dir2 definitions around for userspace xfs: split out attribute listing code into separate file xfs: split out attribute fork truncation code into separate file xfs: split out the remote symlink handling xfs: introduce xfs_sb.c for sharing with libxfs xfs: create xfs_bmap_util.[ch] xfs: minor cleanups xfs: fix issues that cause userspace warnings xfs: kill xfs_vnodeops.[ch] xfs: consolidate xfs_rename.c xfs: consolidate xfs_utils.c xfs: consolidate extent swap code xfs: don't special case shared superblock mounts xfs: kill __KERNEL__ check for debug code in allocation code xfs: remove __KERNEL__ from debug code xfs: remove __KERNEL__ check from xfs_dir2_leaf.c xfs: xfs_filestreams.h doesn't need __KERNEL__ xfs: move kernel specific type definitions to xfs.h xfs: make struct xfs_perag kernel only xfs: return log item size in IOP_SIZE xfs: Reduce allocations during CIL insertion xfs: avoid CIL allocation during insert xfs: Combine CIL insert and prepare passes xfs: split the CIL lock xfs: use reference counts to free clean buffer items xfs: Add read-only support for dirent filetype field xfs: Add write support for dirent filetype field xfs: don't account buffer cancellation during log recovery readahead xfs: fix bad dquot buffer size in log recovery readahead XFS: Assertion failed: first <= last && last < BBTOB(bp->b_length), file: fs/xfs/xfs_trans_buf.c, line: 568 xfs: btree block LSN escaping to disk uninitialised xfs: check LSN ordering for v5 superblocks during recovery xfs: inode buffers may not be valid during recovery readahead xfs: inode log reservations are too small xfs: finish removing IOP_* macros. xfs: dtype changed xfs_dir2_sfe_put_ino to xfs_dir3_sfe_put_ino xfs: XFS_MOUNT_QUOTA_ALL needed by userspace Dwight Engen (8):
[GIT PULL] XFS updates for 3.12-rc1
Hi Linus, Please pull these XFS updates for 3.12-rc1. Most of this is related to the libxfs kernel/userspace sync, there is also project quota work, performance work in the log, recovery, and the CIL. User namespace support has been added, directory entries now have file type support, there is work to cleanup log space reservations, a bunch of spelling cleanups, and bug fixes. Thanks, Ben The following changes since commit ad81f0545ef01ea651886dddac4bef6cec930092: Linux 3.11-rc1 (2013-07-14 15:18:27 -0700) are available in the git repository at: git://oss.sgi.com/xfs/xfs tags/xfs-for-linus-v3.12-rc1 for you to fetch changes up to 1d03c6fa88af35e55047a1f2ab116f0fdf2f55aa: xfs: XFS_MOUNT_QUOTA_ALL needed by userspace (2013-09-03 15:00:06 -0500) xfs: update for v3.12-rc1 For 3.12-rc1 there are a number of bugfixes in addition to work to ease usage of shared code between libxfs and the kernel, the rest of the work to enable project and group quotas to be used simultaneously, performance optimisations in the log and the CIL, directory entry file type support, fixes for log space reservations, some spelling/grammar cleanups, and the addition of user namespace support. - introduce readahead to log recovery - add directory entry file type support - fix a number of spelling errors in comments - introduce new Q_XGETQSTATV quotactl for project quotas - add USER_NS support - log space reservation rework - CIL optimisations - kernel/userspace libxfs rework Brian Foster (1): xfs: check correct status variable for xfs_inobt_get_rec() call Chandra Seetharaman (5): xfs: Fix a deadlock in xfs_log_commit_cil() code path xfs: Initialize all quota inodes to be NULLFSINO xfs: Start using pquotaino from the superblock. quota: Add a new quotactl command Q_XGETQSTATV xfs: Add support for the Q_XGETQSTATV Dan Carpenter (1): xfs: check for underflow in xfs_iformat_fork() Dave Chinner (53): xfs: di_flushiter considered harmful xfs: separate out log format definitions xfs: split out inode log item format definition xfs: split out buf log item format definitions xfs: split out EFI/EFD log item format definition xfs: separate dquot on disk format definitions out of xfs_quota.h xfs: separate icreate log format definitions from xfs_icreate_item.h xfs: split out on-disk transaction definitions xfs: introduce xfs_rtalloc_defs.h xfs: introduce xfs_quota_defs.h xfs: sync minor header differences needed by userspace. xfs: split out transaction reservation code xfs: move inode fork definitions to a new header file xfs: move unrelated definitions out of xfs_inode.h xfs: introduce xfs_inode_buf.c for inode buffer operations xfs: move getdents code into it's own file xfs: reshuffle dir2 definitions around for userspace xfs: split out attribute listing code into separate file xfs: split out attribute fork truncation code into separate file xfs: split out the remote symlink handling xfs: introduce xfs_sb.c for sharing with libxfs xfs: create xfs_bmap_util.[ch] xfs: minor cleanups xfs: fix issues that cause userspace warnings xfs: kill xfs_vnodeops.[ch] xfs: consolidate xfs_rename.c xfs: consolidate xfs_utils.c xfs: consolidate extent swap code xfs: don't special case shared superblock mounts xfs: kill __KERNEL__ check for debug code in allocation code xfs: remove __KERNEL__ from debug code xfs: remove __KERNEL__ check from xfs_dir2_leaf.c xfs: xfs_filestreams.h doesn't need __KERNEL__ xfs: move kernel specific type definitions to xfs.h xfs: make struct xfs_perag kernel only xfs: return log item size in IOP_SIZE xfs: Reduce allocations during CIL insertion xfs: avoid CIL allocation during insert xfs: Combine CIL insert and prepare passes xfs: split the CIL lock xfs: use reference counts to free clean buffer items xfs: Add read-only support for dirent filetype field xfs: Add write support for dirent filetype field xfs: don't account buffer cancellation during log recovery readahead xfs: fix bad dquot buffer size in log recovery readahead XFS: Assertion failed: first = last last BBTOB(bp-b_length), file: fs/xfs/xfs_trans_buf.c, line: 568 xfs: btree block LSN escaping to disk uninitialised xfs: check LSN ordering for v5 superblocks during recovery xfs: inode buffers may not be valid during recovery readahead xfs: inode log reservations are too small xfs: finish removing IOP_* macros. xfs: dtype changed xfs_dir2_sfe_put_ino to xfs_dir3_sfe_put_ino xfs: XFS_MOUNT_QUOTA_ALL needed by userspace Dwight Engen (8): xfs:
Re: [patch] xfs: check for underflow in xfs_iformat_fork()
Hey Dan,
On Mon, Aug 26, 2013 at 05:37:15PM +0300, Dan Carpenter wrote:
> On Fri, Aug 23, 2013 at 12:36:13PM -0500, Ben Myers wrote:
> > Dan,
> >
> > On Fri, Aug 16, 2013 at 08:26:50AM +1000, Dave Chinner wrote:
> > > On Thu, Aug 15, 2013 at 09:37:06AM -0500, Ben Myers wrote:
> > > > Hey Dan & Jeff,
> > > >
> > > > On Thu, Aug 15, 2013 at 06:10:43PM +0800, Jeff Liu wrote:
> > > > > On 08/15/2013 01:53 PM, Dan Carpenter wrote:
> > > > >
> > > > > > The "di_size" variable comes from the disk and it's a signed 64 bit.
> > > > > > We check the upper limit but we should check for negative numbers as
> > > > > > well.
> > > > > >
> > > > > > Signed-off-by: Dan Carpenter
> > > > > >
> > > > > > diff --git a/fs/xfs/xfs_inode_fork.c b/fs/xfs/xfs_inode_fork.c
> > > > > > index 123971b..849fc70 100644
> > > > > > --- a/fs/xfs/xfs_inode_fork.c
> > > > > > +++ b/fs/xfs/xfs_inode_fork.c
> > > > > > @@ -167,7 +167,8 @@ xfs_iformat_fork(
> > > > > > }
> > > > > >
> > > > > > di_size = be64_to_cpu(dip->di_size);
> > > > > > - if (unlikely(di_size > XFS_DFORK_DSIZE(dip,
> > > > > > ip->i_mount))) {
> > > > > > + if (unlikely(di_size < 0 ||
> > > > >
> > > > > But the di_size is initialized to ZERO while allocating a new inode
> > > > > on disk.
> > > > > I wonder if that is better to ASSERT in this case because the current
> > > > > check
> > > > > is used to make sure that the item is inlined, or we don't need it at
> > > > > all.
> > > >
> > > > Hmm. Dan's additional check looks good to me. In this case I'd say
> > > > the forced
> > > > shutdown is more appropriate than an assert, because here we're reading
> > > > the
> > > > inode from disk, as opposed to looking at a structure that is already
> > > > incore
> > > > which we think we've initialized. We want to handle unexpected inputs
> > > > from
> > > > disk without crashing even if we are CONFIG_XFS_DEBUG.
> > >
> > > There are lots of places where we only check di_size to be greater
> > > than some value, and don't check for it being less than zero. Hence
> > > I think that a better solution might be to di_size unsigned as that
> > > will catch "negative" sizes for all types of situations.
> >
> > What do you say to making di_size unsigned? Any interest?
> >
>
> I'm not the right person to change "lots of places". Some of these
> are probably subtle. Just give me the reported-by and I'm happy.
I'll apply this for now, and we'll see if someone is interested enough to pick
up the rest.
Thanks,
Ben
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] xfs: check for underflow in xfs_iformat_fork()
Hey Dan,
On Mon, Aug 26, 2013 at 05:37:15PM +0300, Dan Carpenter wrote:
On Fri, Aug 23, 2013 at 12:36:13PM -0500, Ben Myers wrote:
Dan,
On Fri, Aug 16, 2013 at 08:26:50AM +1000, Dave Chinner wrote:
On Thu, Aug 15, 2013 at 09:37:06AM -0500, Ben Myers wrote:
Hey Dan Jeff,
On Thu, Aug 15, 2013 at 06:10:43PM +0800, Jeff Liu wrote:
On 08/15/2013 01:53 PM, Dan Carpenter wrote:
The di_size variable comes from the disk and it's a signed 64 bit.
We check the upper limit but we should check for negative numbers as
well.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/fs/xfs/xfs_inode_fork.c b/fs/xfs/xfs_inode_fork.c
index 123971b..849fc70 100644
--- a/fs/xfs/xfs_inode_fork.c
+++ b/fs/xfs/xfs_inode_fork.c
@@ -167,7 +167,8 @@ xfs_iformat_fork(
}
di_size = be64_to_cpu(dip-di_size);
- if (unlikely(di_size XFS_DFORK_DSIZE(dip,
ip-i_mount))) {
+ if (unlikely(di_size 0 ||
But the di_size is initialized to ZERO while allocating a new inode
on disk.
I wonder if that is better to ASSERT in this case because the current
check
is used to make sure that the item is inlined, or we don't need it at
all.
Hmm. Dan's additional check looks good to me. In this case I'd say
the forced
shutdown is more appropriate than an assert, because here we're reading
the
inode from disk, as opposed to looking at a structure that is already
incore
which we think we've initialized. We want to handle unexpected inputs
from
disk without crashing even if we are CONFIG_XFS_DEBUG.
There are lots of places where we only check di_size to be greater
than some value, and don't check for it being less than zero. Hence
I think that a better solution might be to di_size unsigned as that
will catch negative sizes for all types of situations.
What do you say to making di_size unsigned? Any interest?
I'm not the right person to change lots of places. Some of these
are probably subtle. Just give me the reported-by and I'm happy.
I'll apply this for now, and we'll see if someone is interested enough to pick
up the rest.
Thanks,
Ben
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4] xfs: introduce object readahead to log recovery
On Wed, Aug 14, 2013 at 03:16:03PM +0800, zwu.ker...@gmail.com wrote: > From: Zhi Yong Wu > > It can take a long time to run log recovery operation because it is > single threaded and is bound by read latency. We can find that it took > most of the time to wait for the read IO to occur, so if one object > readahead is introduced to log recovery, it will obviously reduce the > log recovery time. > > Log recovery time stat: > > w/o this patchw/ this patch > > real:0m15.023s 0m7.802s > user:0m0.001s 0m0.001s > sys: 0m0.246s 0m0.107s > > Signed-off-by: Zhi Yong Wu Looks good. Reviewed-by: Ben Myers Applied. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] xfs: check for underflow in xfs_iformat_fork()
Dan,
On Fri, Aug 16, 2013 at 08:26:50AM +1000, Dave Chinner wrote:
> On Thu, Aug 15, 2013 at 09:37:06AM -0500, Ben Myers wrote:
> > Hey Dan & Jeff,
> >
> > On Thu, Aug 15, 2013 at 06:10:43PM +0800, Jeff Liu wrote:
> > > On 08/15/2013 01:53 PM, Dan Carpenter wrote:
> > >
> > > > The "di_size" variable comes from the disk and it's a signed 64 bit.
> > > > We check the upper limit but we should check for negative numbers as
> > > > well.
> > > >
> > > > Signed-off-by: Dan Carpenter
> > > >
> > > > diff --git a/fs/xfs/xfs_inode_fork.c b/fs/xfs/xfs_inode_fork.c
> > > > index 123971b..849fc70 100644
> > > > --- a/fs/xfs/xfs_inode_fork.c
> > > > +++ b/fs/xfs/xfs_inode_fork.c
> > > > @@ -167,7 +167,8 @@ xfs_iformat_fork(
> > > > }
> > > >
> > > > di_size = be64_to_cpu(dip->di_size);
> > > > - if (unlikely(di_size > XFS_DFORK_DSIZE(dip,
> > > > ip->i_mount))) {
> > > > + if (unlikely(di_size < 0 ||
> > >
> > > But the di_size is initialized to ZERO while allocating a new inode on
> > > disk.
> > > I wonder if that is better to ASSERT in this case because the current
> > > check
> > > is used to make sure that the item is inlined, or we don't need it at all.
> >
> > Hmm. Dan's additional check looks good to me. In this case I'd say the
> > forced
> > shutdown is more appropriate than an assert, because here we're reading the
> > inode from disk, as opposed to looking at a structure that is already incore
> > which we think we've initialized. We want to handle unexpected inputs from
> > disk without crashing even if we are CONFIG_XFS_DEBUG.
>
> There are lots of places where we only check di_size to be greater
> than some value, and don't check for it being less than zero. Hence
> I think that a better solution might be to di_size unsigned as that
> will catch "negative" sizes for all types of situations.
What do you say to making di_size unsigned? Any interest?
Thanks,
Ben
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] xfs: check for underflow in xfs_iformat_fork()
Dan,
On Fri, Aug 16, 2013 at 08:26:50AM +1000, Dave Chinner wrote:
On Thu, Aug 15, 2013 at 09:37:06AM -0500, Ben Myers wrote:
Hey Dan Jeff,
On Thu, Aug 15, 2013 at 06:10:43PM +0800, Jeff Liu wrote:
On 08/15/2013 01:53 PM, Dan Carpenter wrote:
The di_size variable comes from the disk and it's a signed 64 bit.
We check the upper limit but we should check for negative numbers as
well.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/fs/xfs/xfs_inode_fork.c b/fs/xfs/xfs_inode_fork.c
index 123971b..849fc70 100644
--- a/fs/xfs/xfs_inode_fork.c
+++ b/fs/xfs/xfs_inode_fork.c
@@ -167,7 +167,8 @@ xfs_iformat_fork(
}
di_size = be64_to_cpu(dip-di_size);
- if (unlikely(di_size XFS_DFORK_DSIZE(dip,
ip-i_mount))) {
+ if (unlikely(di_size 0 ||
But the di_size is initialized to ZERO while allocating a new inode on
disk.
I wonder if that is better to ASSERT in this case because the current
check
is used to make sure that the item is inlined, or we don't need it at all.
Hmm. Dan's additional check looks good to me. In this case I'd say the
forced
shutdown is more appropriate than an assert, because here we're reading the
inode from disk, as opposed to looking at a structure that is already incore
which we think we've initialized. We want to handle unexpected inputs from
disk without crashing even if we are CONFIG_XFS_DEBUG.
There are lots of places where we only check di_size to be greater
than some value, and don't check for it being less than zero. Hence
I think that a better solution might be to di_size unsigned as that
will catch negative sizes for all types of situations.
What do you say to making di_size unsigned? Any interest?
Thanks,
Ben
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4] xfs: introduce object readahead to log recovery
On Wed, Aug 14, 2013 at 03:16:03PM +0800, zwu.ker...@gmail.com wrote: From: Zhi Yong Wu wu...@linux.vnet.ibm.com It can take a long time to run log recovery operation because it is single threaded and is bound by read latency. We can find that it took most of the time to wait for the read IO to occur, so if one object readahead is introduced to log recovery, it will obviously reduce the log recovery time. Log recovery time stat: w/o this patchw/ this patch real:0m15.023s 0m7.802s user:0m0.001s 0m0.001s sys: 0m0.246s 0m0.107s Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com Looks good. Reviewed-by: Ben Myers b...@sgi.com Applied. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xfs: Register hotcpu notifier after initialization
On Mon, Aug 19, 2013 at 05:21:29PM -0500, Ben Myers wrote: > On Mon, Aug 19, 2013 at 10:56:44PM +0200, Richard Weinberger wrote: > > Currently the code initializizes mp->m_icsb_mutex and other things > > _after_ register_hotcpu_notifier(). > > As the notifier takes mp->m_icsb_mutex it can happen > > that it takes the lock before it's initialization. > > > > Signed-off-by: Richard Weinberger > > Looks good. Thanks Richard. > > Reviewed-by: Ben Myers Applied. Thanks Richard. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xfs: Register hotcpu notifier after initialization
On Mon, Aug 19, 2013 at 05:21:29PM -0500, Ben Myers wrote: On Mon, Aug 19, 2013 at 10:56:44PM +0200, Richard Weinberger wrote: Currently the code initializizes mp-m_icsb_mutex and other things _after_ register_hotcpu_notifier(). As the notifier takes mp-m_icsb_mutex it can happen that it takes the lock before it's initialization. Signed-off-by: Richard Weinberger rich...@nod.at Looks good. Thanks Richard. Reviewed-by: Ben Myers b...@sgi.com Applied. Thanks Richard. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] powerpc/spufs: convert userns uid/gid mount options to kuid/kgid
On Wed, Aug 21, 2013 at 10:05:27PM +0200, Arnd Bergmann wrote: > On Wednesday 21 August 2013, Dwight Engen wrote: > > > > Acked-by: Jeremy Kerr > > Tested-by: Jeremy Kerr > > Signed-off-by: Dwight Engen > > Reviewed-by: Arnd Bergmann Applied. Thanks, -Ben -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the final tree
Hey Dwight, On Wed, Aug 21, 2013 at 02:30:04PM +0800, Jeremy Kerr wrote: > > Yes, I agree. The other filesystems that take an Opt_uid as well do use > > current_user_ns() and not init_user_ns. They also do a uid_valid() > > check and fail the mount (or fallback to GLOBAL_ROOT_UID). So I think > > that would look like this: > > Looks good to me. Builds and mounts as expected. > > Acked-by: Jeremy Kerr Could you repost this patch with the right subject and a commit header? Given Jeremy's Ack I think we could proceed to pull this in. Regards, Ben -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the final tree
Hey Stephen, On Wed, Aug 21, 2013 at 10:22:46AM +1000, Stephen Rothwell wrote: > On Tue, 20 Aug 2013 14:28:44 -0500 Ben Myers wrote: > > I'd prefer not to break Stephen's tree two days in a row. We could just > > revert > > d6970d4b726c in the xfs tree for the time being as Stephen has done, but > > given > > the choice would prefer the fix. Do you have a preference between the two > > approaches that Dwight has posted? The first seems more conservative... > > I will automatically revert that commit when I merge the xfs tree until > some other solution is forthcoming (so you don't have to do the revert in > the xfs tree). Gah. That makes sense. ;) > This does need to be fixed fairly soon, though. Agreed, thanks. -Ben -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the final tree
Hey Stephen, On Wed, Aug 21, 2013 at 10:22:46AM +1000, Stephen Rothwell wrote: On Tue, 20 Aug 2013 14:28:44 -0500 Ben Myers b...@sgi.com wrote: I'd prefer not to break Stephen's tree two days in a row. We could just revert d6970d4b726c in the xfs tree for the time being as Stephen has done, but given the choice would prefer the fix. Do you have a preference between the two approaches that Dwight has posted? The first seems more conservative... I will automatically revert that commit when I merge the xfs tree until some other solution is forthcoming (so you don't have to do the revert in the xfs tree). Gah. That makes sense. ;) This does need to be fixed fairly soon, though. Agreed, thanks. -Ben -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the final tree
Hey Dwight, On Wed, Aug 21, 2013 at 02:30:04PM +0800, Jeremy Kerr wrote: Yes, I agree. The other filesystems that take an Opt_uid as well do use current_user_ns() and not init_user_ns. They also do a uid_valid() check and fail the mount (or fallback to GLOBAL_ROOT_UID). So I think that would look like this: Looks good to me. Builds and mounts as expected. Acked-by: Jeremy Kerr j...@ozlabs.org Could you repost this patch with the right subject and a commit header? Given Jeremy's Ack I think we could proceed to pull this in. Regards, Ben -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] powerpc/spufs: convert userns uid/gid mount options to kuid/kgid
On Wed, Aug 21, 2013 at 10:05:27PM +0200, Arnd Bergmann wrote: On Wednesday 21 August 2013, Dwight Engen wrote: Acked-by: Jeremy Kerr j...@ozlabs.org Tested-by: Jeremy Kerr j...@ozlabs.org Signed-off-by: Dwight Engen dwight.en...@oracle.com Reviewed-by: Arnd Bergmann a...@arndb.de Applied. Thanks, -Ben -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the final tree
Hi Jeremy,
Apologies for breaking your build...
On Tue, Aug 20, 2013 at 12:07:02PM -0400, Dwight Engen wrote:
> On Tue, 20 Aug 2013 17:20:52 +1000
> Stephen Rothwell wrote:
> > After merging the final tree, today's linux-next build (powerpc
> > allyesconfig) failed like this:
> >
> > arch/powerpc/platforms/cell/spufs/inode.c: In function
> > 'spufs_parse_options':
> > arch/powerpc/platforms/cell/spufs/inode.c:623:16: error: incompatible
> > types when assigning to type 'kuid_t' from type 'int' root->i_uid =
> > option; ^ arch/powerpc/platforms/cell/spufs/inode.c:628:16: error:
> > incompatible types when assigning to type 'kgid_t' from type 'int'
> > root->i_gid = option; ^
> >
> > Caused by commit d6970d4b726c ("enable building user namespace with
> > xfs") from the xfs tree (that was fun to find :-)).
> >
> > I have reverted that commit for today. It could probably be replaced
> > with a patch that just changed XFS_FS to SPU_FS in the
> > UIDGID_CONVERTED config dependency - or someone could fix up SPU_FS.
>
> Hi, (already sent this email based on Intel's kbuild robot this
> morning, sorry for the dup to those who already got it).
>
> Yep this looks to me like SPU_FS should have been in the list of
> stuff that had not been UIDGID_CONVERTED, but reviving
> UIDGID_CONVERTED and adding SPU_FS to it won't work for
> non powerpc arch because SPU_FS = n won't be defined. The following can
> be used to mark it as incompatible with USER_NS:
>
> diff --git a/arch/powerpc/platforms/cell/Kconfig
> b/arch/powerpc/platforms/cell/Kconfig index 9978f59..fcf8336 100644
> --- a/arch/powerpc/platforms/cell/Kconfig
> +++ b/arch/powerpc/platforms/cell/Kconfig
> @@ -61,6 +61,7 @@ config SPU_FS
> tristate "SPU file system"
> default m
> depends on PPC_CELL
> + depends on USER_NS=n
> select SPU_BASE
> select MEMORY_HOTPLUG
> help
>
> Or if the rest of spufs is already okay for user namespace (I have not
> checked it, but this seems to be the only place it is dealing with
> uid/gid), then the following will fix these particular errors
> (cross-compile tested, but I don't have a powerpc to run test on):
>
> diff --git a/arch/powerpc/platforms/cell/spufs/inode.c
> b/arch/powerpc/platforms/cell/spufs/inode.c
> index f390042..90fb308 100644
> --- a/arch/powerpc/platforms/cell/spufs/inode.c
> +++ b/arch/powerpc/platforms/cell/spufs/inode.c
> @@ -620,12 +620,12 @@ spufs_parse_options(struct super_block *sb, char
> *options, struct inode *root)
> case Opt_uid:
> if (match_int([0], ))
> return 0;
> - root->i_uid = option;
> + root->i_uid = make_kuid(_user_ns, option);
> break;
> case Opt_gid:
> if (match_int([0], ))
> return 0;
> - root->i_gid = option;
> + root->i_gid = make_kgid(_user_ns, option);
> break;
> case Opt_mode:
> if (match_octal([0], ))
I'd prefer not to break Stephen's tree two days in a row. We could just revert
d6970d4b726c in the xfs tree for the time being as Stephen has done, but given
the choice would prefer the fix. Do you have a preference between the two
approaches that Dwight has posted? The first seems more conservative...
Thanks,
Ben
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the final tree
Hi Jeremy, Apologies for breaking your build... On Tue, Aug 20, 2013 at 12:07:02PM -0400, Dwight Engen wrote: On Tue, 20 Aug 2013 17:20:52 +1000 Stephen Rothwell s...@canb.auug.org.au wrote: After merging the final tree, today's linux-next build (powerpc allyesconfig) failed like this: arch/powerpc/platforms/cell/spufs/inode.c: In function 'spufs_parse_options': arch/powerpc/platforms/cell/spufs/inode.c:623:16: error: incompatible types when assigning to type 'kuid_t' from type 'int' root-i_uid = option; ^ arch/powerpc/platforms/cell/spufs/inode.c:628:16: error: incompatible types when assigning to type 'kgid_t' from type 'int' root-i_gid = option; ^ Caused by commit d6970d4b726c (enable building user namespace with xfs) from the xfs tree (that was fun to find :-)). I have reverted that commit for today. It could probably be replaced with a patch that just changed XFS_FS to SPU_FS in the UIDGID_CONVERTED config dependency - or someone could fix up SPU_FS. Hi, (already sent this email based on Intel's kbuild robot this morning, sorry for the dup to those who already got it). Yep this looks to me like SPU_FS should have been in the list of stuff that had not been UIDGID_CONVERTED, but reviving UIDGID_CONVERTED and adding SPU_FS to it won't work for non powerpc arch because SPU_FS = n won't be defined. The following can be used to mark it as incompatible with USER_NS: diff --git a/arch/powerpc/platforms/cell/Kconfig b/arch/powerpc/platforms/cell/Kconfig index 9978f59..fcf8336 100644 --- a/arch/powerpc/platforms/cell/Kconfig +++ b/arch/powerpc/platforms/cell/Kconfig @@ -61,6 +61,7 @@ config SPU_FS tristate SPU file system default m depends on PPC_CELL + depends on USER_NS=n select SPU_BASE select MEMORY_HOTPLUG help Or if the rest of spufs is already okay for user namespace (I have not checked it, but this seems to be the only place it is dealing with uid/gid), then the following will fix these particular errors (cross-compile tested, but I don't have a powerpc to run test on): diff --git a/arch/powerpc/platforms/cell/spufs/inode.c b/arch/powerpc/platforms/cell/spufs/inode.c index f390042..90fb308 100644 --- a/arch/powerpc/platforms/cell/spufs/inode.c +++ b/arch/powerpc/platforms/cell/spufs/inode.c @@ -620,12 +620,12 @@ spufs_parse_options(struct super_block *sb, char *options, struct inode *root) case Opt_uid: if (match_int(args[0], option)) return 0; - root-i_uid = option; + root-i_uid = make_kuid(init_user_ns, option); break; case Opt_gid: if (match_int(args[0], option)) return 0; - root-i_gid = option; + root-i_gid = make_kgid(init_user_ns, option); break; case Opt_mode: if (match_octal(args[0], option)) I'd prefer not to break Stephen's tree two days in a row. We could just revert d6970d4b726c in the xfs tree for the time being as Stephen has done, but given the choice would prefer the fix. Do you have a preference between the two approaches that Dwight has posted? The first seems more conservative... Thanks, Ben -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xfs: Register hotcpu notifier after initialization
On Mon, Aug 19, 2013 at 10:56:44PM +0200, Richard Weinberger wrote: > Currently the code initializizes mp->m_icsb_mutex and other things > _after_ register_hotcpu_notifier(). > As the notifier takes mp->m_icsb_mutex it can happen > that it takes the lock before it's initialization. > > Signed-off-by: Richard Weinberger Looks good. Thanks Richard. Reviewed-by: Ben Myers -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xfs: Register hotcpu notifier after initialization
On Mon, Aug 19, 2013 at 10:56:44PM +0200, Richard Weinberger wrote: Currently the code initializizes mp-m_icsb_mutex and other things _after_ register_hotcpu_notifier(). As the notifier takes mp-m_icsb_mutex it can happen that it takes the lock before it's initialization. Signed-off-by: Richard Weinberger rich...@nod.at Looks good. Thanks Richard. Reviewed-by: Ben Myers b...@sgi.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] xfs: check for underflow in xfs_iformat_fork()
Hey Dan & Jeff,
On Thu, Aug 15, 2013 at 06:10:43PM +0800, Jeff Liu wrote:
> On 08/15/2013 01:53 PM, Dan Carpenter wrote:
>
> > The "di_size" variable comes from the disk and it's a signed 64 bit.
> > We check the upper limit but we should check for negative numbers as
> > well.
> >
> > Signed-off-by: Dan Carpenter
> >
> > diff --git a/fs/xfs/xfs_inode_fork.c b/fs/xfs/xfs_inode_fork.c
> > index 123971b..849fc70 100644
> > --- a/fs/xfs/xfs_inode_fork.c
> > +++ b/fs/xfs/xfs_inode_fork.c
> > @@ -167,7 +167,8 @@ xfs_iformat_fork(
> > }
> >
> > di_size = be64_to_cpu(dip->di_size);
> > - if (unlikely(di_size > XFS_DFORK_DSIZE(dip,
> > ip->i_mount))) {
> > + if (unlikely(di_size < 0 ||
>
> But the di_size is initialized to ZERO while allocating a new inode on disk.
> I wonder if that is better to ASSERT in this case because the current check
> is used to make sure that the item is inlined, or we don't need it at all.
Hmm. Dan's additional check looks good to me. In this case I'd say the forced
shutdown is more appropriate than an assert, because here we're reading the
inode from disk, as opposed to looking at a structure that is already incore
which we think we've initialized. We want to handle unexpected inputs from
disk without crashing even if we are CONFIG_XFS_DEBUG.
How did you come across this one?
Reviewed-by: Ben Myers
Regards,
Ben
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] xfs: check for underflow in xfs_iformat_fork()
Hey Dan Jeff,
On Thu, Aug 15, 2013 at 06:10:43PM +0800, Jeff Liu wrote:
On 08/15/2013 01:53 PM, Dan Carpenter wrote:
The di_size variable comes from the disk and it's a signed 64 bit.
We check the upper limit but we should check for negative numbers as
well.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/fs/xfs/xfs_inode_fork.c b/fs/xfs/xfs_inode_fork.c
index 123971b..849fc70 100644
--- a/fs/xfs/xfs_inode_fork.c
+++ b/fs/xfs/xfs_inode_fork.c
@@ -167,7 +167,8 @@ xfs_iformat_fork(
}
di_size = be64_to_cpu(dip-di_size);
- if (unlikely(di_size XFS_DFORK_DSIZE(dip,
ip-i_mount))) {
+ if (unlikely(di_size 0 ||
But the di_size is initialized to ZERO while allocating a new inode on disk.
I wonder if that is better to ASSERT in this case because the current check
is used to make sure that the item is inlined, or we don't need it at all.
Hmm. Dan's additional check looks good to me. In this case I'd say the forced
shutdown is more appropriate than an assert, because here we're reading the
inode from disk, as opposed to looking at a structure that is already incore
which we think we've initialized. We want to handle unexpected inputs from
disk without crashing even if we are CONFIG_XFS_DEBUG.
How did you come across this one?
Reviewed-by: Ben Myers b...@sgi.com
Regards,
Ben
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] xfs: Implement FALLOC_FL_COLLAPSE_RANGE
Hey Namjae,
On Wed, Jul 31, 2013 at 11:42:14PM +0900, Namjae Jeon wrote:
> From: Namjae Jeon
>
> New fallocate flag FALLOC_FL_COLLAPSE_RANGE implementation for XFS.
>
> Signed-off-by: Namjae Jeon
> Signed-off-by: Ashish Sangwan
Very cool feature! ;)
I have a couple initial suggestions/questions:
> ---
> fs/xfs/xfs_bmap.c | 92
> +
> fs/xfs/xfs_bmap.h |3 ++
> fs/xfs/xfs_file.c | 26 --
> fs/xfs/xfs_iops.c | 35 +++
> fs/xfs/xfs_vnodeops.c | 62 +
> fs/xfs/xfs_vnodeops.h |2 ++
> 6 files changed, 217 insertions(+), 3 deletions(-)
>
> diff --git a/fs/xfs/xfs_bmap.c b/fs/xfs/xfs_bmap.c
> index 05c698c..ae677b1 100644
> --- a/fs/xfs/xfs_bmap.c
> +++ b/fs/xfs/xfs_bmap.c
> @@ -6145,3 +6145,95 @@ next_block:
>
> return error;
> }
> +
> +/*
> + * xfs_update_logical()
> + * Updates starting logical block of extents by subtracting
> + * shift from them. At max XFS_LINEAR_EXTS number extents
> + * will be updated and *current_ext is the extent number which
> + * is currently being updated.
> + */
> +int
> +xfs_update_logical(
> + xfs_trans_t *tp,
> + struct xfs_inode*ip,
> + int *done,
> + xfs_fileoff_t start_fsb,
> + xfs_fileoff_t shift,
> + xfs_extnum_t*current_ext)
Could we find a better name for this function? Maybe something like
xfs_shift_extent_startblocks or xfs_shift_extent_offsets?
Also, is there also a case for being able to shift extent offsets upward in the
file's address space so that you can splice in a chunk of data? For that I
think maybe you'd want to be able to shift on sub-block boundaries too, and
there would be some copying and zeroing involved on the boundary block. Not
sure.
> +{
> + xfs_btree_cur_t *cur;
> + xfs_bmbt_rec_host_t *gotp;
> + xfs_mount_t *mp;
> + xfs_ifork_t *ifp;
> + xfs_extnum_tnexts = 0;
> + xfs_fileoff_t startoff;
> + int error = 0;
> + int i;
> +
> + ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
> + mp = ip->i_mount;
> +
> + if (!(ifp->if_flags & XFS_IFEXTENTS) &&
> + (error = xfs_iread_extents(tp, ip, XFS_DATA_FORK)))
> + return error;
> +
> + if (!*current_ext) {
> + gotp = xfs_iext_bno_to_ext(ifp, start_fsb, current_ext);
> + /*
> + * gotp can be null in 2 cases: 1) if there are no extents
> + * or 2) start_fsb lies in a hole beyond which there are
> + * no extents. Either way, we are done.
> + */
> + if (!gotp) {
> + *done = 1;
> + return 0;
> + }
> + }
> +
> + if (ifp->if_flags & XFS_IFBROOT)
> + cur = xfs_bmbt_init_cursor(mp, tp, ip, XFS_DATA_FORK);
> + else
> + cur = NULL;
> +
> + while (nexts++ < XFS_LINEAR_EXTS &&
> +*current_ext < XFS_IFORK_NEXTENTS(ip, XFS_DATA_FORK)) {
> + gotp = xfs_iext_get_ext(ifp, (*current_ext)++);
> + startoff = xfs_bmbt_get_startoff(gotp);
> + if (cur) {
> + if ((error = xfs_bmbt_lookup_eq(cur,
> + startoff,
> + xfs_bmbt_get_startblock(gotp),
> + xfs_bmbt_get_blockcount(gotp),
> + )))
> + goto del_cursor;
> + XFS_WANT_CORRUPTED_GOTO(i == 1, del_cursor);
> + }
> + startoff -= shift;
> + xfs_bmbt_set_startoff(gotp, startoff);
> + if (cur) {
> + error = xfs_bmbt_update(cur, startoff,
> + xfs_bmbt_get_startblock(gotp),
> + xfs_bmbt_get_blockcount(gotp),
> + xfs_bmbt_get_state(gotp));
> + if (error)
> + goto del_cursor;
> + }
> + }
> +
> + /* Check if we are done */
> + if (*current_ext == XFS_IFORK_NEXTENTS(ip, XFS_DATA_FORK))
> + *done = 1;
> +
> +del_cursor:
> + if (cur) {
> + if (!error)
> + cur->bc_private.b.allocated = 0;
> + xfs_btree_del_cursor(cur,
> + error ? XFS_BTREE_ERROR : XFS_BTREE_NOERROR);
> + }
> +
> + xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE | XFS_ILOG_DEXT);
> +
> + return error;
> +}
> diff --git a/fs/xfs/xfs_bmap.h b/fs/xfs/xfs_bmap.h
> index 1cf1292..f923734 100644
> --- a/fs/xfs/xfs_bmap.h
> +++ b/fs/xfs/xfs_bmap.h
> @@ -204,6 +204,9 @@ int xfs_bunmapi(struct xfs_trans *tp, struct
Re: [PATCH v2] xfs: introduce object readahead to log recovery
Hey Zhi,
On Wed, Jul 31, 2013 at 12:07:32PM +0800, Zhi Yong Wu wrote:
> On Wed, Jul 31, 2013 at 7:11 AM, Dave Chinner wrote:
> > On Tue, Jul 30, 2013 at 05:59:07PM +0800, zwu.ker...@gmail.com wrote:
> >> From: Zhi Yong Wu
> >>
> >> It can take a long time to run log recovery operation because it is
> >> single threaded and is bound by read latency. We can find that it took
> >> most of the time to wait for the read IO to occur, so if one object
> >> readahead is introduced to log recovery, it will obviously reduce the
> >> log recovery time.
> >>
> >> Log recovery time stat:
> >>
> >> w/o this patchw/ this patch
> >>
> >> real:0m15.023s 0m7.802s
> >> user:0m0.001s 0m0.001s
> >> sys: 0m0.246s 0m0.107s
> >>
> >> Signed-off-by: Zhi Yong Wu
> >> ---
> >> fs/xfs/xfs_log_recover.c | 162
> >> +--
> >> fs/xfs/xfs_log_recover.h | 2 +
> >> 2 files changed, 159 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> >> index 7681b19..029826f 100644
> >> --- a/fs/xfs/xfs_log_recover.c
> >> +++ b/fs/xfs/xfs_log_recover.c
> >> @@ -3116,6 +3116,111 @@ xlog_recover_free_trans(
> >> kmem_free(trans);
> >> }
> >>
> >> +STATIC void
> >> +xlog_recover_buffer_ra_pass2(
> >> + struct xlog *log,
> >> + struct xlog_recover_item*item)
> >> +{
> >> + xfs_buf_log_format_t*buf_f = item->ri_buf[0].i_addr;
> >> + xfs_mount_t *mp = log->l_mp;
> >
> > struct xfs_buf_log_format
> > struct xfs_mount
> Why? *_t is also used in a lot of other places.
It is just a general style preference for using the struct instead of the _t in
the xfs codebase. Over the course of the past few years they've slowly been
converted in this direction, and we prefer not to add any more _t if it can be
avoided.
-Ben
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
