Re: [PATCH 00/18] Rid W=1 warnings from GFS2
- Original Message - > On Fri, 26 Mar 2021, Lee Jones wrote: > > > This set is part of a larger effort attempting to clean-up W=1 > > kernel builds, which are currently overwhelmingly riddled with > > niggly little warnings. > > > > Lee Jones (18): > > fs: gfs2: dir: Finish off mostly complete headers demote others > > fs: gfs2: xattr: Help out some kernel-doc header demote non-conformant > > ones > > fs: gfs2: bmap: Demote half-complete kernel-doc headers fix others > > fs: gfs2: glops: Fix a couple of kernel-doc misdemeanours demote > > others > > fs: gfs2: log: Fix a couple of param descriptions and demote > > non-conformant headers > > fs: gfs2: lops: Help out worthy kernel-doc headers and demote others > > fs: gfs2: glock: Fix some deficient kernel-doc headers and demote > > non-conformant ones > > fs: gfs2: aops: Fix a little potential doc-rot > > fs: gfs2: meta_io: Add missing description for 'rahead' param > > fs: gfs2: inode: Fix worthy function headers demote others > > fs: gfs2: file: Strip out superflouous param description and demote > > kernel-doc abuse > > fs: gfs2: ops_fstype: Fix a little doc-rot > > fs: gfs2: quota: Fix incorrect param name in 'gfs2_quotad' > > fs: gfs2: rgrp: Fix a few kernel-doc misdemeanours > > fs: gfs2: recovery: Provide missing param descriptions and remove one > > other > > fs: gfs2: super: Fix kernel-doc issues where feasible demote one other > > fs: gfs2: util: Fix one conformant kernel-doc header and demote a > > bunch of abuses > > fs: gfs2: lock_dlm: Demote incomplete kernel-doc header > > These have been on the list for a couple of weeks now. > > Is there anything I can do to help expedite their merge? > > I'm concerned since -rc6 has just been released. Hi Lee, Thanks for your patches. Andreas and I have a slight disagreement about them. I wanted to merge them, but Andreas is concerned that there may be some cases in which we should get rid of parameter descriptions altogether. So I've been waiting for Andreas to review them all before we push them. Also, the dir.c patch might be out of date because of a patch I recently did that was recently added to for-next. I'll work with Andreas to expedite them so they can go into the next merge window. Regards, Bob Peterson
Re: [PATCH] gfs2: make function gfs2_make_fs_ro() to void type
- Original Message - > It fixes the following warning detected by coccinelle: > ./fs/gfs2/super.c:592:5-10: Unneeded variable: "error". Return "0" on > line 628 > > Reported-by: Abaci Robot > Signed-off-by: Yang Li > --- > fs/gfs2/ops_fstype.c | 4 +--- > fs/gfs2/super.c | 9 +++-- > fs/gfs2/super.h | 2 +- > fs/gfs2/util.c | 2 +- > 4 files changed, 6 insertions(+), 11 deletions(-) Thanks. Your patch is now pushed to linux-gfs2/for-next. I did one minor cleanup: Function gfs2_put_super in super.c no longer needed the variable "error" so I removed it to avoid warnings. https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/fs/gfs2?h=for-next=0e787a0bfabb8be323f575d5cdda48e607840eb5 Regards, Bob Peterson
Re: [PATCH] gfs2: Remove unneeded return variable
- Original Message -
> This patch removes unneeded return variables, using only
> '0' instead.
> It fixes the following warning detected by coccinelle:
> ./fs/gfs2/super.c:592:5-10: Unneeded variable: "error". Return "0" on
> line 628
>
> Reported-by: Abaci Robot
> Signed-off-by: Yang Li
> ---
> fs/gfs2/super.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> index 861ed5f..fe2dae4 100644
> --- a/fs/gfs2/super.c
> +++ b/fs/gfs2/super.c
> @@ -589,7 +589,6 @@ static void gfs2_dirty_inode(struct inode *inode, int
> flags)
>
> int gfs2_make_fs_ro(struct gfs2_sbd *sdp)
> {
> - int error = 0;
> int log_write_allowed = test_bit(SDF_JOURNAL_LIVE, >sd_flags);
>
> gfs2_flush_delete_work(sdp);
> @@ -625,7 +624,7 @@ int gfs2_make_fs_ro(struct gfs2_sbd *sdp)
> if (!log_write_allowed)
> sdp->sd_vfs->s_flags |= SB_RDONLY;
>
> - return error;
> + return 0;
> }
>
> /**
> --
> 1.8.3.1
Hi Yang,
Thanks for submitting your patch. I like it. However, since gfs2_make_fs_ro
always returns 0, we should also be able to make it a void function instead
of int, and change its callers to not act on any return code.
Regards,
Bob Peterson
Red Hat File Systems
Re: [PATCH] fs: Fix freeze_bdev()/thaw_bdev() accounting of bd_fsfreeze_sb
Hi, > This is the bigger issue, and I'm not very familiar with this code either, > so I'll defer to the experts. Yes, it's a change in behavior, but I think > it makes sense to decrement the bd_fsfreeze_count in this case. Here's why: > > If the blockdev is frozen by freeze_bdev while it's being unmounted, the > bd_fsfreeze_count is incremented, but the freeze is ignored. Subsequent > attempts to thaw the device will be ignored but return 0 because the sb > is not found. When the device is mounted again, calls to freeze_bdev > will bypass the call to freeze_super for the newly mounted sb, because > bdev->bd_fsfreeze_count was then incremented from 1 to 2 in freeze_bdev. > > if (++bdev->bd_fsfreeze_count > 1) > goto done; > > So you're freezing the device without really freezing the superblock. > Seems like dangerous behavior to me. The new sb will only be frozen if > a second thaw is done, which gets them back in sync. I suppose we could > say this is acceptable loss, and your number of thaws should match your > freezes, and if they don't: user error. Still, it seems like we should do > something about it, like refuse to mount a frozen device. Perhaps it already > does that; I'll need to do some research. After some experiments, I've determined that my fears about the count are unfounded. Consider my patch withdrawn. Sorry for the noise. Bob Peterson
Re: [PATCH] fs: Fix freeze_bdev()/thaw_bdev() accounting of bd_fsfreeze_sb
- Original Message - > This causes bdev->bd_fsfreeze_sb to be set to NULL even if the call to > thaw_super right after this line fail. So if a caller tries to call > thaw_bdev() again after receiving such an error, that next call won't even > try to call thaw_super(). Is that what we want here? (I don't know much > about this code, but from a cursory glance I think this difference is > visible to emergency_thaw_bdev() in fs/buffer.c) > > In my version of the patch, I set bdev->bd_fsfreeze_sb to NULL only > *after* we check that the call to thaw_super() succeeded to avoid this. Yes, I see your point. Your patch is superior and I'll mine accordingly. > Thanks a lot for investigating the bug and the patch I sent :) > Was there actually an issue with that patch I sent? As you said, the bug No, I never saw your patch until I saw Christoph's reference to it yesterday, after I had been using my patch to fix the problem. AFAIK, there is no problem with your patch. > I think the second difference (decrementing bd_fsfreeze_count when > get_active_super() returns NULL) doesn't change anything w.r.t the > use-after-free. It does however, change the behaviour of the function > slightly, and it might be caller visible (because from a cursory glance, it > looks like we're reading the bd_fsfreeze_count from some other places like > fs/super.c). Even before 040f04bd2e82, the code wouldn't decrement > bd_fsfreeze_count when get_active_super() returned NULL - so is this change > in behaviour intentional? And if so, maybe it should go in a separate > patch? This is the bigger issue, and I'm not very familiar with this code either, so I'll defer to the experts. Yes, it's a change in behavior, but I think it makes sense to decrement the bd_fsfreeze_count in this case. Here's why: If the blockdev is frozen by freeze_bdev while it's being unmounted, the bd_fsfreeze_count is incremented, but the freeze is ignored. Subsequent attempts to thaw the device will be ignored but return 0 because the sb is not found. When the device is mounted again, calls to freeze_bdev will bypass the call to freeze_super for the newly mounted sb, because bdev->bd_fsfreeze_count was then incremented from 1 to 2 in freeze_bdev. if (++bdev->bd_fsfreeze_count > 1) goto done; So you're freezing the device without really freezing the superblock. Seems like dangerous behavior to me. The new sb will only be frozen if a second thaw is done, which gets them back in sync. I suppose we could say this is acceptable loss, and your number of thaws should match your freezes, and if they don't: user error. Still, it seems like we should do something about it, like refuse to mount a frozen device. Perhaps it already does that; I'll need to do some research. Like I said, I don't know this code. I'm just trying to fix a problem I observed. I'll defer to the experts. Regards, Bob Peterson
[fs PATCH] fs: fix freeze count problem in freeze_bdev
Hi,
I wrote this patch to fix the freeze/thaw device problem before I saw
the patch "fs: Fix freeze_bdev()/thaw_bdev() accounting of bd_fsfreeze_sb"
from Satya Tangirala. That one, however, does not fix the bd_freeze_count
problem and this patch does. Jens, Christoph, what do you think?
This is a very recreatable problem via repeated runs of generic/085,
at least on gfs2.
Description:
Before this patch, if you tried to freeze a device (function freeze_bdev)
while it was being unmounted, it would get NULL back from get_active_super
and correctly bypass the freeze calls. Unfortunately, it forgot to decrement
its bd_fsfreeze_count. Subsequent calls to device thaw (thaw_bdev) would
see the non-zero bd_fsfreeze_count and assume the bd_fsfreeze_sb value was
still valid. That's not a safe assumption and resulted in use-after-free,
which often caused fatal kernel errors like: "unable to handle page fault
for address."
This patch adds the necessary decrement of bd_fsfreeze_count for that
error path. It also adds code to set the bd_fsfreeze_sb to NULL when the
last reference is reached in thaw_bdev.
Signed-off-by: Bob Peterson
---
fs/block_dev.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 9e56ee1f2652..c6daf7d12546 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -555,8 +555,10 @@ int freeze_bdev(struct block_device *bdev)
goto done;
sb = get_active_super(bdev);
- if (!sb)
+ if (!sb) {
+ bdev->bd_fsfreeze_count--;
goto sync;
+ }
if (sb->s_op->freeze_super)
error = sb->s_op->freeze_super(sb);
else
@@ -600,6 +602,7 @@ int thaw_bdev(struct block_device *bdev)
if (!sb)
goto out;
+ bdev->bd_fsfreeze_sb = NULL;
if (sb->s_op->thaw_super)
error = sb->s_op->thaw_super(sb);
else
Re: [PATCH] fs: Fix freeze_bdev()/thaw_bdev() accounting of bd_fsfreeze_sb
- Original Message -
> Can someone pick this up? Maybe through Jens' block tree as that is
> where my commit this is fixing up came from.
Christoph and Al,
Here is my version:
Bob Peterson
fs: fix freeze count problem in freeze_bdev
Before this patch, if you tried to freeze a device (function freeze_bdev)
while it was being unmounted, it would get NULL back from get_active_super
and correctly bypass the freeze calls. Unfortunately, it forgot to decrement
its bd_fsfreeze_count. Subsequent calls to device thaw (thaw_bdev) would
see the non-zero bd_fsfreeze_count and assume the bd_fsfreeze_sb value was
still valid. That's not a safe assumption and resulted in use-after-free,
which often caused fatal kernel errors like: "unable to handle page fault
for address."
This patch adds the necessary decrement of bd_fsfreeze_count for that
error path. It also adds code to set the bd_fsfreeze_sb to NULL when the
last reference is reached in thaw_bdev.
Reviewed-by: Bob Peterson
---
fs/block_dev.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 9e56ee1f2652..c6daf7d12546 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -555,8 +555,10 @@ int freeze_bdev(struct block_device *bdev)
goto done;
sb = get_active_super(bdev);
- if (!sb)
+ if (!sb) {
+ bdev->bd_fsfreeze_count--;
goto sync;
+ }
if (sb->s_op->freeze_super)
error = sb->s_op->freeze_super(sb);
else
@@ -600,6 +602,7 @@ int thaw_bdev(struct block_device *bdev)
if (!sb)
goto out;
+ bdev->bd_fsfreeze_sb = NULL;
if (sb->s_op->thaw_super)
error = sb->s_op->thaw_super(sb);
else
Re: [PATCH] fs: Fix freeze_bdev()/thaw_bdev() accounting of bd_fsfreeze_sb
- Original Message -
> Can someone pick this up? Maybe through Jens' block tree as that is
> where my commit this is fixing up came from.
>
> For reference:
>
>
> Reviewed-by: Christoph Hellwig
>
> On Thu, Dec 24, 2020 at 04:49:54AM +, Satya Tangirala wrote:
> > freeze/thaw_bdev() currently use bdev->bd_fsfreeze_count to infer
> > whether or not bdev->bd_fsfreeze_sb is valid (it's valid iff
> > bd_fsfreeze_count is non-zero). thaw_bdev() doesn't nullify
> > bd_fsfreeze_sb.
> >
> > But this means a freeze_bdev() call followed by a thaw_bdev() call can
> > leave bd_fsfreeze_sb with a non-null value, while bd_fsfreeze_count is
> > zero. If freeze_bdev() is called again, and this time
> > get_active_super() returns NULL (e.g. because the FS is unmounted),
> > we'll end up with bd_fsfreeze_count > 0, but bd_fsfreeze_sb is
> > *untouched* - it stays the same (now garbage) value. A subsequent
> > thaw_bdev() will decide that the bd_fsfreeze_sb value is legitimate
> > (since bd_fsfreeze_count > 0), and attempt to use it.
> >
> > Fix this by always setting bd_fsfreeze_sb to NULL when
> > bd_fsfreeze_count is successfully decremented to 0 in thaw_sb().
> > Alternatively, we could set bd_fsfreeze_sb to whatever
> > get_active_super() returns in freeze_bdev() whenever bd_fsfreeze_count
> > is successfully incremented to 1 from 0 (which can be achieved cleanly
> > by moving the line currently setting bd_fsfreeze_sb to immediately
> > after the "sync:" label, but it might be a little too subtle/easily
> > overlooked in future).
> >
> > This fixes the currently panicking xfstests generic/085.
> >
> > Fixes: 040f04bd2e82 ("fs: simplify freeze_bdev/thaw_bdev")
> > Signed-off-by: Satya Tangirala
> > ---
> > fs/block_dev.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/fs/block_dev.c b/fs/block_dev.c
> > index 9e56ee1f2652..12a811a9ae4b 100644
> > --- a/fs/block_dev.c
> > +++ b/fs/block_dev.c
> > @@ -606,6 +606,8 @@ int thaw_bdev(struct block_device *bdev)
> > error = thaw_super(sb);
> > if (error)
> > bdev->bd_fsfreeze_count++;
> > + else
> > + bdev->bd_fsfreeze_sb = NULL;
> > out:
> > mutex_unlock(>bd_fsfreeze_mutex);
> > return error;
> > --
> > 2.29.2.729.g45daf8777d-goog
> ---end quoted text---
>
>
Funny you should ask. I came across this bug in my testing of gfs2
and my patch is slightly different. I was wondering who to send it to.
Perhaps Viro?
Regards,
Bob Peterson
Re: [PATCH v2] fs: amend SLAB_RECLAIM_ACCOUNT on gfs2 related slab cache
- Original Message - > From: Zhaoyang Huang > > As gfs2_quotad_cachep and gfs2_glock_cachep have registered > the shrinker, amending SLAB_RECLAIM_ACCOUNT when creating > them, which make the slab acount to be presiced. > > Signed-off-by: Zhaoyang Huang > --- > v2: add gfs2_glock_cachep for same operation > --- Hi, Thanks. Your patch is now pushed to the linux-gfs2/for-next branch. I cleaned up the description a bit. For example, I changed "fs:" to "gfs2:" to conform to other gfs2 patches. Regards, Bob Peterson
Re: [PATCH] fs: amend SLAB_RECLAIM_ACCOUNT on gfs2_quotad_cachep
- Original Message -
> From: Zhaoyang Huang
>
> As gfs2_quotad_cachep has registered the shrinker, amending
> SLAB_RECLAIM_ACCOUNT when create gfs2_quotad_cachep, which
> make the slab acount to be presiced.
>
> Signed-off-by: Zhaoyang Huang
> ---
> fs/gfs2/main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/gfs2/main.c b/fs/gfs2/main.c
> index 136484e..db39de9 100644
> --- a/fs/gfs2/main.c
> +++ b/fs/gfs2/main.c
> @@ -136,7 +136,7 @@ static int __init init_gfs2_fs(void)
>
> gfs2_quotad_cachep = kmem_cache_create("gfs2_quotad",
> sizeof(struct gfs2_quota_data),
> -0, 0, NULL);
> +0, SLAB_RECLAIM_ACCOUNT, NULL);
> if (!gfs2_quotad_cachep)
> goto fail_cachep6;
>
> --
> 1.9.1
>
>
Hi,
Thanks for the patch.
We should also do this for gfs2_glock_cachep. Can you add that to your patch?
Regards,
Bob Peterson
Re: [Cluster-devel] general protection fault in gfs2_withdraw
- Original Message -
> On 26/09/2020 18:21, syzbot wrote:
> > syzbot has found a reproducer for the following issue on:
> >
> > HEAD commit:7c7ec322 Merge tag 'for-linus' of
> > git://git.kernel.org/pub..
> > git tree: upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=11f2ff2790
> > kernel config: https://syzkaller.appspot.com/x/.config?x=6184b75aa6d48d66
> > dashboard link:
> > https://syzkaller.appspot.com/bug?extid=50a8a9cf8127f2c6f5df
> > compiler: clang version 10.0.0 (https://github.com/llvm/llvm-project/
> > c2443155a0fb245c8f17f2c1c72b6ea391e86e81)
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=160fb77390
> > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1104f10990
> >
> > IMPORTANT: if you fix the issue, please add the following tag to the
> > commit:
> > Reported-by: syzbot+50a8a9cf8127f2c6f...@syzkaller.appspotmail.com
> >
> > gfs2: fsid=syz:syz.0: fatal: invalid metadata block
> >bh = 2072 (magic number)
> >function = gfs2_meta_indirect_buffer, file = fs/gfs2/meta_io.c, line =
> >417
> > gfs2: fsid=syz:syz.0: about to withdraw this file system
> > general protection fault, probably for non-canonical address
> > 0xdc0e: [#1] PREEMPT SMP KASAN
> > KASAN: null-ptr-deref in range [0x0070-0x0077]
> > CPU: 0 PID: 6842 Comm: syz-executor264 Not tainted 5.9.0-rc6-syzkaller #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > Google 01/01/2011
> > RIP: 0010:signal_our_withdraw fs/gfs2/util.c:97 [inline]
>
> Seems that it's withdrawing in the init_inodes() path early enough
> (while looking up the jindex) that sdp->sd_jdesc is still NULL here:
>
>static void signal_our_withdraw(struct gfs2_sbd *sdp)
>{
>struct gfs2_glock *gl = sdp->sd_live_gh.gh_gl;
>struct inode *inode = sdp->sd_jdesc->jd_inode;
>
> I'm undecided as to whether the bug is that we're withdrawing that early
> at all, or that we're not checking for NULL there?
>
> Probably introduced by:
>
> 601ef0d52e96 gfs2: Force withdraw to replay journals and wait for it to
> finish
>
> Andy
Hi Andy. Thanks for your analysis.
I suspect you're right.
It's probably another exception to the rule. We knew there would be a few of
those with 601ef0d52e96, such as the one we made for "withdrawing during
withdraw".
We should probably just add a check for NULL and make it do the right thing.
Regards,
Bob Peterson
Re: [PATCH 4.19 142/206] gfs2: fix use-after-free on transaction ail lists
- Original Message -
> Hi Bob, hi Greg,
>
> On Fri, Sep 11, 2020 at 08:49:14AM -0400, Bob Peterson wrote:
> > - Original Message -
> > > On Fri, Sep 11, 2020 at 08:08:35AM -0400, Bob Peterson wrote:
> > > > - Original Message -
> > > > > On Thu, Sep 10, 2020 at 09:43:19PM +0200, Salvatore Bonaccorso wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On Tue, Jun 23, 2020 at 09:57:50PM +0200, Greg Kroah-Hartman wrote:
> > > > > > > From: Bob Peterson
> > > > > > >
> > > > > > > [ Upstream commit 83d060ca8d90fa1e3feac227f995c013100862d3 ]
> > > > > > >
> > > > > > > Before this patch, transactions could be merged into the system
> > > > > > > transaction by function gfs2_merge_trans(), but the transaction
> > > > > > > ail
> > > > > > > lists were never merged. Because the ail flushing mechanism can
> > > > > > > run
> > > > > > > separately, bd elements can be attached to the transaction's
> > > > > > > buffer
> > > > > > > list during the transaction (trans_add_meta, etc) but quickly
> > > > > > > moved
> > > > > > > to its ail lists. Later, in function gfs2_trans_end, the
> > > > > > > transaction
> > > > > > > can be freed (by gfs2_trans_end) while it still has bd elements
> > > > > > > queued to its ail lists, which can cause it to either lose track
> > > > > > > of
> > > > > > > the bd elements altogether (memory leak) or worse, reference the
> > > > > > > bd
> > > > > > > elements after the parent transaction has been freed.
> > > > > > >
> > > > > > > Although I've not seen any serious consequences, the problem
> > > > > > > becomes
> > > > > > > apparent with the previous patch's addition of:
> > > > > > >
> > > > > > > gfs2_assert_warn(sdp, list_empty(>tr_ail1_list));
> > > > > > >
> > > > > > > to function gfs2_trans_free().
> > > > > > >
> > > > > > > This patch adds logic into gfs2_merge_trans() to move the merged
> > > > > > > transaction's ail lists to the sdp transaction. This prevents the
> > > > > > > use-after-free. To do this properly, we need to hold the ail
> > > > > > > lock,
> > > > > > > so we pass sdp into the function instead of the transaction
> > > > > > > itself.
> > > > > > >
> > > > > > > Signed-off-by: Bob Peterson
> > > > > > > Signed-off-by: Andreas Gruenbacher
> > > > > > > Signed-off-by: Sasha Levin
> > > > (snip)
> > > > > >
> > > > > > In Debian two user confirmed issues on writing on a GFS2 partition
> > > > > > with this commit applied. The initial Debian report is at
> > > > > > https://bugs.debian.org/968567 and Daniel Craig reported it into
> > > > > > Bugzilla at https://bugzilla.kernel.org/show_bug.cgi?id=209217 .
> > > > > >
> > > > > > Writing to a gfs2 filesystem fails and results in a soft lookup of
> > > > > > the
> > > > > > machine for kernels with that commit applied. I cannot reporduce
> > > > > > the
> > > > > > issue myself due not having a respective setup available, but
> > > > > > Daniel
> > > > > > described a minimal serieos of steps to reproduce the issue.
> > > > > >
> > > > > > This might affect as well other stable series where this commit was
> > > > > > applied, as there was a similar report for someone running 5.4.58
> > > > > > in
> > > > > > https://www.redhat.com/archives/linux-cluster/2020-August/msg0.html
> > > > >
> > > > > Can you report this to the gfs2 developers?
> > > > >
> > > > > thanks,
> > > > >
> > > > > greg k-h
> > > >
> > > > Hi Greg,
> > > >
> > > > No need. The patch came from the gfs2 developers. I think he just wants
> > > > it added to a stable release.
> > >
> > > What commit needs to be added to a stable release?
> > >
> > > confused,
> > >
> > > greg k-h
> >
> > Sorry Greg,
> >
> > It's pretty early here and the caffeine hadn't quite hit my system.
> > The problem is most likely that 4.19.132 is missing this upstream patch:
> >
> > cbcc89b630447ec7836aa2b9242d9bb1725f5a61
> >
> > I'm not sure how or why 83d060ca8d90fa1e3feac227f995c013100862d3 got
> > put into stable without a stable CC but cbcc89b6304 is definitely
> > required.
> >
> > I'd like to suggest Salvatore try cherry-picking this patch to see if
> > it fixes the problem, and if so, perhaps Greg can add it to stable.
>
> I can confirm (Daniel was able to test): Applying cbcc89b63044 ("gfs2:
> initialize transaction tr_ailX_lists earlier") fixes the issue. So
> would be great if you can pick that up for stable for those series
> which had 83d060ca8d90 ("gfs2: fix use-after-free on transaction ail
> lists") as well.
>
> Regards,
> Salvatore
>
>
Hi Greg,
As per Salvatore's email above, can you please cherry-pick GFS2 patch
cbcc89b630447ec7836aa2b9242d9bb1725f5a61 to the stable releases like
4.19 to which ("gfs2: fix use-after-free on transaction ail lists")
(83d060ca8d90fa1e3feac227f995c013100862d3) was applied? Thanks.
Regards,
Bob Peterson
GFS2 File System
Re: [PATCH 4.19 142/206] gfs2: fix use-after-free on transaction ail lists
- Original Message - > On Fri, Sep 11, 2020 at 08:08:35AM -0400, Bob Peterson wrote: > > - Original Message - > > > On Thu, Sep 10, 2020 at 09:43:19PM +0200, Salvatore Bonaccorso wrote: > > > > Hi, > > > > > > > > On Tue, Jun 23, 2020 at 09:57:50PM +0200, Greg Kroah-Hartman wrote: > > > > > From: Bob Peterson > > > > > > > > > > [ Upstream commit 83d060ca8d90fa1e3feac227f995c013100862d3 ] > > > > > > > > > > Before this patch, transactions could be merged into the system > > > > > transaction by function gfs2_merge_trans(), but the transaction ail > > > > > lists were never merged. Because the ail flushing mechanism can run > > > > > separately, bd elements can be attached to the transaction's buffer > > > > > list during the transaction (trans_add_meta, etc) but quickly moved > > > > > to its ail lists. Later, in function gfs2_trans_end, the transaction > > > > > can be freed (by gfs2_trans_end) while it still has bd elements > > > > > queued to its ail lists, which can cause it to either lose track of > > > > > the bd elements altogether (memory leak) or worse, reference the bd > > > > > elements after the parent transaction has been freed. > > > > > > > > > > Although I've not seen any serious consequences, the problem becomes > > > > > apparent with the previous patch's addition of: > > > > > > > > > > gfs2_assert_warn(sdp, list_empty(>tr_ail1_list)); > > > > > > > > > > to function gfs2_trans_free(). > > > > > > > > > > This patch adds logic into gfs2_merge_trans() to move the merged > > > > > transaction's ail lists to the sdp transaction. This prevents the > > > > > use-after-free. To do this properly, we need to hold the ail lock, > > > > > so we pass sdp into the function instead of the transaction itself. > > > > > > > > > > Signed-off-by: Bob Peterson > > > > > Signed-off-by: Andreas Gruenbacher > > > > > Signed-off-by: Sasha Levin > > (snip) > > > > > > > > In Debian two user confirmed issues on writing on a GFS2 partition > > > > with this commit applied. The initial Debian report is at > > > > https://bugs.debian.org/968567 and Daniel Craig reported it into > > > > Bugzilla at https://bugzilla.kernel.org/show_bug.cgi?id=209217 . > > > > > > > > Writing to a gfs2 filesystem fails and results in a soft lookup of the > > > > machine for kernels with that commit applied. I cannot reporduce the > > > > issue myself due not having a respective setup available, but Daniel > > > > described a minimal serieos of steps to reproduce the issue. > > > > > > > > This might affect as well other stable series where this commit was > > > > applied, as there was a similar report for someone running 5.4.58 in > > > > https://www.redhat.com/archives/linux-cluster/2020-August/msg0.html > > > > > > Can you report this to the gfs2 developers? > > > > > > thanks, > > > > > > greg k-h > > > > Hi Greg, > > > > No need. The patch came from the gfs2 developers. I think he just wants > > it added to a stable release. > > What commit needs to be added to a stable release? > > confused, > > greg k-h Sorry Greg, It's pretty early here and the caffeine hadn't quite hit my system. The problem is most likely that 4.19.132 is missing this upstream patch: cbcc89b630447ec7836aa2b9242d9bb1725f5a61 I'm not sure how or why 83d060ca8d90fa1e3feac227f995c013100862d3 got put into stable without a stable CC but cbcc89b6304 is definitely required. I'd like to suggest Salvatore try cherry-picking this patch to see if it fixes the problem, and if so, perhaps Greg can add it to stable. Regards, Bob Peterson GFS2 File Systems
Re: [PATCH 4.19 142/206] gfs2: fix use-after-free on transaction ail lists
- Original Message - > On Thu, Sep 10, 2020 at 09:43:19PM +0200, Salvatore Bonaccorso wrote: > > Hi, > > > > On Tue, Jun 23, 2020 at 09:57:50PM +0200, Greg Kroah-Hartman wrote: > > > From: Bob Peterson > > > > > > [ Upstream commit 83d060ca8d90fa1e3feac227f995c013100862d3 ] > > > > > > Before this patch, transactions could be merged into the system > > > transaction by function gfs2_merge_trans(), but the transaction ail > > > lists were never merged. Because the ail flushing mechanism can run > > > separately, bd elements can be attached to the transaction's buffer > > > list during the transaction (trans_add_meta, etc) but quickly moved > > > to its ail lists. Later, in function gfs2_trans_end, the transaction > > > can be freed (by gfs2_trans_end) while it still has bd elements > > > queued to its ail lists, which can cause it to either lose track of > > > the bd elements altogether (memory leak) or worse, reference the bd > > > elements after the parent transaction has been freed. > > > > > > Although I've not seen any serious consequences, the problem becomes > > > apparent with the previous patch's addition of: > > > > > > gfs2_assert_warn(sdp, list_empty(>tr_ail1_list)); > > > > > > to function gfs2_trans_free(). > > > > > > This patch adds logic into gfs2_merge_trans() to move the merged > > > transaction's ail lists to the sdp transaction. This prevents the > > > use-after-free. To do this properly, we need to hold the ail lock, > > > so we pass sdp into the function instead of the transaction itself. > > > > > > Signed-off-by: Bob Peterson > > > Signed-off-by: Andreas Gruenbacher > > > Signed-off-by: Sasha Levin (snip) > > > > In Debian two user confirmed issues on writing on a GFS2 partition > > with this commit applied. The initial Debian report is at > > https://bugs.debian.org/968567 and Daniel Craig reported it into > > Bugzilla at https://bugzilla.kernel.org/show_bug.cgi?id=209217 . > > > > Writing to a gfs2 filesystem fails and results in a soft lookup of the > > machine for kernels with that commit applied. I cannot reporduce the > > issue myself due not having a respective setup available, but Daniel > > described a minimal serieos of steps to reproduce the issue. > > > > This might affect as well other stable series where this commit was > > applied, as there was a similar report for someone running 5.4.58 in > > https://www.redhat.com/archives/linux-cluster/2020-August/msg0.html > > Can you report this to the gfs2 developers? > > thanks, > > greg k-h Hi Greg, No need. The patch came from the gfs2 developers. I think he just wants it added to a stable release. Regards, Bob Peterson GFS2 File System
GFS2: Pull request (merge window)
Hi Linus, Please consider pulling the following changes for the GFS2 file system. Regards, Bob Peterson The following changes since commit 49a57857aeea06ca831043acbb0fa5e0f50602fd: Linux 5.0-rc3 (2019-01-21 13:14:44 +1300) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-5.1.fixes for you to fetch changes up to 605b0487f0bc1ae9963bf52ece0f5c8055186f81: gfs2: Fix missed wakeups in find_insert_glock (2019-03-08 15:49:01 +0100) We've only got three patches ready for this merge window: - Fix a hang related to missed wakeups for glocks from Andreas Gruenbacher. - Rework of how gfs2 manages its debugfs files from Greg K-H. - An incorrect assert when truncating or deleting files from Tim Smith. Andreas Gruenbacher (1): gfs2: Fix missed wakeups in find_insert_glock Greg Kroah-Hartman (1): gfs: no need to check return value of debugfs_create functions Tim Smith (1): gfs2: Fix an incorrect gfs2_assert() fs/gfs2/glock.c | 72 fs/gfs2/glock.h | 4 ++-- fs/gfs2/incore.h | 3 --- fs/gfs2/inode.h | 4 ++-- fs/gfs2/main.c | 6 + 5 files changed, 20 insertions(+), 69 deletions(-)
[PATCH] Revert "gfs2: read journal in large chunks to locate the head"
This reverts commit 2a5f14f279f59143139bcd1606903f2f80a34241.
This patch causes xfstests generic/311 to fail. Reverting this for
now until we have a proper fix.
Signed-off-by: Abhi Das
Signed-off-by: Bob Peterson
---
fs/gfs2/glops.c | 1 -
fs/gfs2/log.c| 4 +-
fs/gfs2/lops.c | 190 ++-
fs/gfs2/lops.h | 4 +-
fs/gfs2/ops_fstype.c | 1 -
fs/gfs2/recovery.c | 123 +
fs/gfs2/recovery.h | 2 +
fs/gfs2/super.c | 1 -
8 files changed, 134 insertions(+), 192 deletions(-)
diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index f15b4c57c4bd..78510ab91835 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -28,7 +28,6 @@
#include "util.h"
#include "trans.h"
#include "dir.h"
-#include "lops.h"
struct workqueue_struct *gfs2_freeze_wq;
diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index 5bfaf381921a..b8830fda51e8 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -733,7 +733,7 @@ void gfs2_write_log_header(struct gfs2_sbd *sdp, struct
gfs2_jdesc *jd,
lh->lh_crc = cpu_to_be32(crc);
gfs2_log_write(sdp, page, sb->s_blocksize, 0, addr);
- gfs2_log_submit_bio(>sd_log_bio, REQ_OP_WRITE | op_flags);
+ gfs2_log_submit_bio(>sd_log_bio, REQ_OP_WRITE, op_flags);
log_flush_wait(sdp);
}
@@ -810,7 +810,7 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock
*gl, u32 flags)
gfs2_ordered_write(sdp);
lops_before_commit(sdp, tr);
- gfs2_log_submit_bio(>sd_log_bio, REQ_OP_WRITE);
+ gfs2_log_submit_bio(>sd_log_bio, REQ_OP_WRITE, 0);
if (sdp->sd_log_head != sdp->sd_log_flush_head) {
log_flush_wait(sdp);
diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
index 94dcab655bc0..2295042bc625 100644
--- a/fs/gfs2/lops.c
+++ b/fs/gfs2/lops.c
@@ -17,9 +17,7 @@
#include
#include
#include
-#include
-#include "bmap.h"
#include "dir.h"
#include "gfs2.h"
#include "incore.h"
@@ -195,6 +193,7 @@ static void gfs2_end_log_write_bh(struct gfs2_sbd *sdp,
struct bio_vec *bvec,
/**
* gfs2_end_log_write - end of i/o to the log
* @bio: The bio
+ * @error: Status of i/o request
*
* Each bio_vec contains either data from the pagecache or data
* relating to the log itself. Here we iterate over the bio_vec
@@ -231,19 +230,20 @@ static void gfs2_end_log_write(struct bio *bio)
/**
* gfs2_log_submit_bio - Submit any pending log bio
* @biop: Address of the bio pointer
- * @opf: REQ_OP | op_flags
+ * @op: REQ_OP
+ * @op_flags: req_flag_bits
*
* Submit any pending part-built or full bio to the block device. If
* there is no pending bio, then this is a no-op.
*/
-void gfs2_log_submit_bio(struct bio **biop, int opf)
+void gfs2_log_submit_bio(struct bio **biop, int op, int op_flags)
{
struct bio *bio = *biop;
if (bio) {
struct gfs2_sbd *sdp = bio->bi_private;
atomic_inc(>sd_log_in_flight);
- bio->bi_opf = opf;
+ bio_set_op_attrs(bio, op, op_flags);
submit_bio(bio);
*biop = NULL;
}
@@ -304,7 +304,7 @@ static struct bio *gfs2_log_get_bio(struct gfs2_sbd *sdp,
u64 blkno,
nblk >>= sdp->sd_fsb2bb_shift;
if (blkno == nblk && !flush)
return bio;
- gfs2_log_submit_bio(biop, op);
+ gfs2_log_submit_bio(biop, op, 0);
}
*biop = gfs2_log_alloc_bio(sdp, blkno, end_io);
@@ -375,184 +375,6 @@ void gfs2_log_write_page(struct gfs2_sbd *sdp, struct
page *page)
gfs2_log_bmap(sdp));
}
-/**
- * gfs2_end_log_read - end I/O callback for reads from the log
- * @bio: The bio
- *
- * Simply unlock the pages in the bio. The main thread will wait on them and
- * process them in order as necessary.
- */
-
-static void gfs2_end_log_read(struct bio *bio)
-{
- struct page *page;
- struct bio_vec *bvec;
- int i;
-
- bio_for_each_segment_all(bvec, bio, i) {
- page = bvec->bv_page;
- if (bio->bi_status) {
- int err = blk_status_to_errno(bio->bi_status);
-
- SetPageError(page);
- mapping_set_error(page->mapping, err);
- }
- unlock_page(page);
- }
-
- bio_put(bio);
-}
-
-/**
- * gfs2_jhead_pg_srch - Look for the journal head in a given page.
- * @jd: The journal descriptor
- * @page: The page to look in
- *
- * Returns: 1 if found, 0 otherwise.
- */
-
-static bool gfs2_jhead_pg_srch(struct gfs2_jdesc *jd,
- struct gfs2_log_header_host *head,
- struct page *page)
-{
- struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
-
GFS2: Pull request (merge window)
Hi Linus,
Please consider pulling the following changes for the GFS2 file system.
Regards,
Bob Peterson
The following changes since commit 40e020c129cfc991e8ab4736d2665351ffd1468d:
Linux 4.20-rc6 (2018-12-09 15:31:00 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git
tags/gfs2-4.21.fixes
for you to fetch changes up to bc0205612bbd4dd4026d4ba6287f5643c37366ec:
gfs2: take jdata unstuff into account in do_grow (2018-12-18 10:49:02 -0600)
We've got 11 patches for this merge window:
- Enhancements and performance improvements to journal replay (Abhi Das)
- Cleanup of gfs2_is_ordered and gfs2_is_writeback (Andreas Gruenbacher)
- Fix a potential double-free in inode creation (Andreas Gruenbacher)
- Fix the bitmap search loop that was searching too far (Andreas Gruenbacher)
- Various cleanups (Andreas Gruenbacher, Bob Peterson)
- Implement Steve Whitehouse's patch to dump nrpages for inodes (Bob Peterson)
- Fix a withdraw bug where stuffed journaled data files didn't allocate
enough journal space to be grown (Bob Peterson)
Abhi Das (4):
gfs2: add more timing info to journal recovery process
gfs2: changes to gfs2_log_XXX_bio
gfs2: add a helper function to get_log_header that can be used elsewhere
gfs2: read journal in large chunks to locate the head
Andreas Gruenbacher (4):
gfs2: Clean up gfs2_is_{ordered,writeback}
gfs2: Fix the gfs2_invalidatepage description
gfs2: Get rid of potential double-freeing in gfs2_create_inode
gfs2: Fix loop in gfs2_rbm_find
Bob Peterson (3):
gfs2: Remove vestigial bd_ops
gfs2: Dump nrpages for inodes and their glocks
gfs2: take jdata unstuff into account in do_grow
fs/gfs2/aops.c | 16 ++--
fs/gfs2/bmap.c | 10 +-
fs/gfs2/glock.c | 2 +-
fs/gfs2/glock.h | 2 +-
fs/gfs2/glops.c | 17 +++-
fs/gfs2/incore.h | 3 +-
fs/gfs2/inode.c | 18 ++--
fs/gfs2/inode.h | 10 +-
fs/gfs2/log.c| 5 +-
fs/gfs2/log.h| 5 +-
fs/gfs2/lops.c | 257 +++
fs/gfs2/lops.h | 4 +-
fs/gfs2/ops_fstype.c | 1 +
fs/gfs2/recovery.c | 178 +++
fs/gfs2/recovery.h | 5 +-
fs/gfs2/rgrp.c | 4 +-
fs/gfs2/rgrp.h | 2 +-
fs/gfs2/super.c | 1 +
fs/gfs2/trans.c | 8 +-
19 files changed, 317 insertions(+), 231 deletions(-)
GFS2: Pull request (merge window)
Hi Linus,
Welcome back.
Please consider pulling the following changes for the GFS2 file system.
Regards,
Bob Peterson
The following changes since commit 050cdc6c9501abcd64720b8cc3e7941efee9547d:
Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2018-08-27
11:59:39 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git
tags/gfs2-4.20.fixes
for you to fetch changes up to 8e31582a9ab18093c0b7708b52d7f79eed9a8688:
gfs2: Fix minor typo: couln't versus couldn't. (2018-10-19 11:34:04 -0500)
We've got 18 patches for this merge window, none of which are very major.
1. Andreas Gruenbacher contributed several patches to clean up the gfs2
block allocator to prepare for future performance enhancements.
2. Andy Price contributed a patch to fix a use-after-free problem.
3. I contributed some patches that fix gfs2's broken rgrplvb mount option.
4. I contributed some cleanup patches and error message improvements.
5. Steve Whitehouse and Abhi Das sent a patch to enable getlabel support.
6. Tim Smith contributed a patch to flush the glock delete workqueue at exit.
Andreas Gruenbacher (8):
gfs2: Always check the result of gfs2_rbm_from_block
gfs2: Clean up out-of-bounds check in gfs2_rbm_from_block
gfs2: Move rs_{sizehint, rgd_gh} fields into the inode
gfs2: Remove unused RGRP_RSRV_MINBYTES definition
gfs2: Rename bitmap.bi_{len => bytes}
gfs2: Fix some minor typos
gfs2: Fix marking bitmaps non-full
gfs2: Pass resource group to rgblk_free
Andrew Price (1):
gfs2: Don't leave s_fs_info pointing to freed memory in init_sbd
Bob Peterson (7):
gfs2: improve debug information when lvb mismatches are found
gfs2: Don't set GFS2_RDF_UPTODATE when the lvb is updated
gfs2: slow the deluge of io error messages
gfs2: Use fs_* functions instead of pr_* function where we can
gfs2: Remove unnecessary gfs2_rlist_alloc parameter
gfs2: write revokes should traverse sd_ail1_list in reverse
gfs2: Fix minor typo: couln't versus couldn't.
Steve Whitehouse (1):
gfs2: getlabel support
Tim Smith (1):
GFS2: Flush the GFS2 delete workqueue before stopping the kernel threads
fs/gfs2/bmap.c | 4 +-
fs/gfs2/dir.c| 28 ---
fs/gfs2/file.c | 18 -
fs/gfs2/glock.c | 17 +++--
fs/gfs2/incore.h | 9 +--
fs/gfs2/lock_dlm.c | 10 +--
fs/gfs2/log.c| 11 ++-
fs/gfs2/lops.c | 2 +-
fs/gfs2/main.c | 2 +
fs/gfs2/ops_fstype.c | 2 +-
fs/gfs2/quota.c | 2 +-
fs/gfs2/rgrp.c | 201 +--
fs/gfs2/rgrp.h | 11 +--
fs/gfs2/super.c | 4 +-
fs/gfs2/trans.c | 15 ++--
fs/gfs2/util.c | 16 ++--
fs/gfs2/util.h | 2 +-
fs/gfs2/xattr.c | 18 +++--
18 files changed, 218 insertions(+), 154 deletions(-)
GFS2: Pull request (merge window)
Hi Linus,
Welcome back.
Please consider pulling the following changes for the GFS2 file system.
Regards,
Bob Peterson
The following changes since commit 050cdc6c9501abcd64720b8cc3e7941efee9547d:
Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2018-08-27
11:59:39 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git
tags/gfs2-4.20.fixes
for you to fetch changes up to 8e31582a9ab18093c0b7708b52d7f79eed9a8688:
gfs2: Fix minor typo: couln't versus couldn't. (2018-10-19 11:34:04 -0500)
We've got 18 patches for this merge window, none of which are very major.
1. Andreas Gruenbacher contributed several patches to clean up the gfs2
block allocator to prepare for future performance enhancements.
2. Andy Price contributed a patch to fix a use-after-free problem.
3. I contributed some patches that fix gfs2's broken rgrplvb mount option.
4. I contributed some cleanup patches and error message improvements.
5. Steve Whitehouse and Abhi Das sent a patch to enable getlabel support.
6. Tim Smith contributed a patch to flush the glock delete workqueue at exit.
Andreas Gruenbacher (8):
gfs2: Always check the result of gfs2_rbm_from_block
gfs2: Clean up out-of-bounds check in gfs2_rbm_from_block
gfs2: Move rs_{sizehint, rgd_gh} fields into the inode
gfs2: Remove unused RGRP_RSRV_MINBYTES definition
gfs2: Rename bitmap.bi_{len => bytes}
gfs2: Fix some minor typos
gfs2: Fix marking bitmaps non-full
gfs2: Pass resource group to rgblk_free
Andrew Price (1):
gfs2: Don't leave s_fs_info pointing to freed memory in init_sbd
Bob Peterson (7):
gfs2: improve debug information when lvb mismatches are found
gfs2: Don't set GFS2_RDF_UPTODATE when the lvb is updated
gfs2: slow the deluge of io error messages
gfs2: Use fs_* functions instead of pr_* function where we can
gfs2: Remove unnecessary gfs2_rlist_alloc parameter
gfs2: write revokes should traverse sd_ail1_list in reverse
gfs2: Fix minor typo: couln't versus couldn't.
Steve Whitehouse (1):
gfs2: getlabel support
Tim Smith (1):
GFS2: Flush the GFS2 delete workqueue before stopping the kernel threads
fs/gfs2/bmap.c | 4 +-
fs/gfs2/dir.c| 28 ---
fs/gfs2/file.c | 18 -
fs/gfs2/glock.c | 17 +++--
fs/gfs2/incore.h | 9 +--
fs/gfs2/lock_dlm.c | 10 +--
fs/gfs2/log.c| 11 ++-
fs/gfs2/lops.c | 2 +-
fs/gfs2/main.c | 2 +
fs/gfs2/ops_fstype.c | 2 +-
fs/gfs2/quota.c | 2 +-
fs/gfs2/rgrp.c | 201 +--
fs/gfs2/rgrp.h | 11 +--
fs/gfs2/super.c | 4 +-
fs/gfs2/trans.c | 15 ++--
fs/gfs2/util.c | 16 ++--
fs/gfs2/util.h | 2 +-
fs/gfs2/xattr.c | 18 +++--
18 files changed, 218 insertions(+), 154 deletions(-)
Re: linux-next: Signed-off-by missing for commit in the gfs2 tree
- Original Message -
> Hi all,
>
> Commit
>
> b7f5a2cd27b7 ("GFS2: Flush the GFS2 delete workqueue before stopping the
> kernel threads")
>
> is missing a Signed-off-by from its committer.
>
> --
> Cheers,
> Stephen Rothwell
Fixed now. Sorry about that.
Regards,
Bob Peterson
Re: linux-next: Signed-off-by missing for commit in the gfs2 tree
- Original Message -
> Hi all,
>
> Commit
>
> b7f5a2cd27b7 ("GFS2: Flush the GFS2 delete workqueue before stopping the
> kernel threads")
>
> is missing a Signed-off-by from its committer.
>
> --
> Cheers,
> Stephen Rothwell
Fixed now. Sorry about that.
Regards,
Bob Peterson
GFS2: Pull request #2 (merge window)
Hi Linus, Please consider pulling the following additional 3 patches for the GFS2 file system. Regards, Bob Peterson The following changes since commit e241e3f2bf975788a1b70dff2eb5180ca395b28e: Merge tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost (2018-04-11 18:58:27 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.17.fixes2 for you to fetch changes up to 3e7aafc39c59c639ebd5961f893743f076df9b4e: GFS2: Minor improvements to comments and documentation (2018-04-12 10:07:51 -0700) We decided to request the latest three patches to be merged into this merge window while it's still open. 1. The first patch adds a new function to lockref: lockref_put_not_zero 2. The second patch fixes GFS2's glock dump code so it uses the new lockref function. This fixes a problem whereby lock dumps could miss glocks. 3. I made a minor patch to update some comments and fix the lock ordering text in our gfs2-glocks.txt Documentation file. Andreas Gruenbacher (2): lockref: Add lockref_put_not_zero gfs2: Stop using rhashtable_walk_peek Bob Peterson (1): GFS2: Minor improvements to comments and documentation Documentation/filesystems/gfs2-glocks.txt | 5 ++-- fs/gfs2/bmap.c| 2 +- fs/gfs2/glock.c | 47 ++- fs/gfs2/ops_fstype.c | 2 +- include/linux/lockref.h | 1 + lib/lockref.c | 28 ++ 6 files changed, 62 insertions(+), 23 deletions(-)
GFS2: Pull request #2 (merge window)
Hi Linus, Please consider pulling the following additional 3 patches for the GFS2 file system. Regards, Bob Peterson The following changes since commit e241e3f2bf975788a1b70dff2eb5180ca395b28e: Merge tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost (2018-04-11 18:58:27 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.17.fixes2 for you to fetch changes up to 3e7aafc39c59c639ebd5961f893743f076df9b4e: GFS2: Minor improvements to comments and documentation (2018-04-12 10:07:51 -0700) We decided to request the latest three patches to be merged into this merge window while it's still open. 1. The first patch adds a new function to lockref: lockref_put_not_zero 2. The second patch fixes GFS2's glock dump code so it uses the new lockref function. This fixes a problem whereby lock dumps could miss glocks. 3. I made a minor patch to update some comments and fix the lock ordering text in our gfs2-glocks.txt Documentation file. Andreas Gruenbacher (2): lockref: Add lockref_put_not_zero gfs2: Stop using rhashtable_walk_peek Bob Peterson (1): GFS2: Minor improvements to comments and documentation Documentation/filesystems/gfs2-glocks.txt | 5 ++-- fs/gfs2/bmap.c| 2 +- fs/gfs2/glock.c | 47 ++- fs/gfs2/ops_fstype.c | 2 +- include/linux/lockref.h | 1 + lib/lockref.c | 28 ++ 6 files changed, 62 insertions(+), 23 deletions(-)
Re: [Cluster-devel] [PATCH v2 0/2] gfs2: Stop using rhashtable_walk_peek
- Original Message - > Here's a second version of the patch (now a patch set) to eliminate > rhashtable_walk_peek in gfs2. > > The first patch introduces lockref_put_not_zero, the inverse of > lockref_get_not_zero. > > The second patch eliminates rhashtable_walk_peek in gfs2. In > gfs2_glock_iter_next, the new lockref function from patch one is used to > drop a lockref count as long as the count doesn't drop to zero. This is > almost always the case; if there is a risk of dropping the last > reference, we must defer that to a work queue because dropping the last > reference may sleep. > > Thanks, > Andreas > > Andreas Gruenbacher (2): > lockref: Add lockref_put_not_zero > gfs2: Stop using rhashtable_walk_peek > > fs/gfs2/glock.c | 47 --- > include/linux/lockref.h | 1 + > lib/lockref.c | 28 > 3 files changed, 57 insertions(+), 19 deletions(-) > > -- > 2.14.3 Hi, Thanks. These two patches are now pushed to the for-next branch of the linux-gfs2 tree: https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/?h=for-next=450b1f6f56350c630e795f240dc5a77aa8aa2419 https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/?h=for-next=3fd5d3ad35dc44aaf0f28d60cc0eb75887bff54d Regards, Bob Peterson Red Hat File Systems
Re: [Cluster-devel] [PATCH v2 0/2] gfs2: Stop using rhashtable_walk_peek
- Original Message - > Here's a second version of the patch (now a patch set) to eliminate > rhashtable_walk_peek in gfs2. > > The first patch introduces lockref_put_not_zero, the inverse of > lockref_get_not_zero. > > The second patch eliminates rhashtable_walk_peek in gfs2. In > gfs2_glock_iter_next, the new lockref function from patch one is used to > drop a lockref count as long as the count doesn't drop to zero. This is > almost always the case; if there is a risk of dropping the last > reference, we must defer that to a work queue because dropping the last > reference may sleep. > > Thanks, > Andreas > > Andreas Gruenbacher (2): > lockref: Add lockref_put_not_zero > gfs2: Stop using rhashtable_walk_peek > > fs/gfs2/glock.c | 47 --- > include/linux/lockref.h | 1 + > lib/lockref.c | 28 > 3 files changed, 57 insertions(+), 19 deletions(-) > > -- > 2.14.3 Hi, Thanks. These two patches are now pushed to the for-next branch of the linux-gfs2 tree: https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/?h=for-next=450b1f6f56350c630e795f240dc5a77aa8aa2419 https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/?h=for-next=3fd5d3ad35dc44aaf0f28d60cc0eb75887bff54d Regards, Bob Peterson Red Hat File Systems
Re: [Cluster-devel] [PATCH v2 0/2] gfs2: Stop using rhashtable_walk_peek
- Original Message - > Here's a second version of the patch (now a patch set) to eliminate > rhashtable_walk_peek in gfs2. > > The first patch introduces lockref_put_not_zero, the inverse of > lockref_get_not_zero. > > The second patch eliminates rhashtable_walk_peek in gfs2. In > gfs2_glock_iter_next, the new lockref function from patch one is used to > drop a lockref count as long as the count doesn't drop to zero. This is > almost always the case; if there is a risk of dropping the last > reference, we must defer that to a work queue because dropping the last > reference may sleep. > > Thanks, > Andreas > > Andreas Gruenbacher (2): > lockref: Add lockref_put_not_zero > gfs2: Stop using rhashtable_walk_peek > > fs/gfs2/glock.c | 47 --- > include/linux/lockref.h | 1 + > lib/lockref.c | 28 > 3 files changed, 57 insertions(+), 19 deletions(-) > > -- > 2.14.3 Hi, The patches look good. The big question is whether to add them to this merge window while it's still open. Opinions? Acked-by: Bob Peterson <rpete...@redhat.com> Regards, Bob Peterson
Re: [Cluster-devel] [PATCH v2 0/2] gfs2: Stop using rhashtable_walk_peek
- Original Message - > Here's a second version of the patch (now a patch set) to eliminate > rhashtable_walk_peek in gfs2. > > The first patch introduces lockref_put_not_zero, the inverse of > lockref_get_not_zero. > > The second patch eliminates rhashtable_walk_peek in gfs2. In > gfs2_glock_iter_next, the new lockref function from patch one is used to > drop a lockref count as long as the count doesn't drop to zero. This is > almost always the case; if there is a risk of dropping the last > reference, we must defer that to a work queue because dropping the last > reference may sleep. > > Thanks, > Andreas > > Andreas Gruenbacher (2): > lockref: Add lockref_put_not_zero > gfs2: Stop using rhashtable_walk_peek > > fs/gfs2/glock.c | 47 --- > include/linux/lockref.h | 1 + > lib/lockref.c | 28 > 3 files changed, 57 insertions(+), 19 deletions(-) > > -- > 2.14.3 Hi, The patches look good. The big question is whether to add them to this merge window while it's still open. Opinions? Acked-by: Bob Peterson Regards, Bob Peterson
GFS2: Pull request (merge window)
Hi Linus, Please consider pulling the following changes for the GFS2 file system. Regards, Bob Peterson The following changes since commit 1b88accf6a659c46d5c8e68912896f112bf882bb: Merge tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost (2018-03-07 17:49:33 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.17.fixes for you to fetch changes up to 5e86d9d122d0d6fae00d9dff41c22d6f4d09f566: gfs2: time journal recovery steps accurately (2018-03-29 10:41:27 -0700) We've only got 9 GFS2 patches for this merge window: 1. Abhi Das contributed a patch to report journal recovery times more accurately during journal replay. 2. Andreas Gruenbacher contributed a patch to fix fallocate chunk size. 3. Andreas added a patch to correctly dirty inodes during rename. 4. Andreas added a patch to improve the comment for function gfs2_block_map. 5. Andreas added a patch to improve kernel trace point iomap end: The physical block address was added. 6. Andreas added a patch to fix a nasty file system corruption bug that surfaced in xfstests 476 in punch-hole/truncate. 7. Andreas fixed a problem Christoph Helwig pointed out, namely, that GFS2 was misusing the IOMAP_ZERO flag. The zeroing of new blocks was moved to the proper fallocate code. 8. I contributed a patch to declare function gfs2_remove_from_ail as static. 9. I added a patch to only set PageChecked for jdata page writes. Abhi Das (1): gfs2: time journal recovery steps accurately Andreas Gruenbacher (6): gfs2: Fix fallocate chunk size gfs2: Dirty source inode during rename gfs2: Improve gfs2_block_map comment gfs2: gfs2_iomap_end tracepoint: log block address gfs2: Check for the end of metadata in punch_hole gfs2: Zero out fallocated blocks in fallocate_chunk Bob Peterson (2): GFS2: Make function gfs2_remove_from_ail static GFS2: Only set PageChecked for jdata pages fs/gfs2/aops.c | 8 fs/gfs2/bmap.c | 38 -- fs/gfs2/dir.c| 13 ++--- fs/gfs2/file.c | 34 +- fs/gfs2/incore.h | 3 --- fs/gfs2/inode.c | 10 +- fs/gfs2/log.c| 2 +- fs/gfs2/log.h| 1 - fs/gfs2/quota.h | 2 ++ fs/gfs2/recovery.c | 20 ++-- fs/gfs2/trace_gfs2.h | 9 +++-- 11 files changed, 68 insertions(+), 72 deletions(-)
GFS2: Pull request (merge window)
Hi Linus, Please consider pulling the following changes for the GFS2 file system. Regards, Bob Peterson The following changes since commit 1b88accf6a659c46d5c8e68912896f112bf882bb: Merge tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost (2018-03-07 17:49:33 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.17.fixes for you to fetch changes up to 5e86d9d122d0d6fae00d9dff41c22d6f4d09f566: gfs2: time journal recovery steps accurately (2018-03-29 10:41:27 -0700) We've only got 9 GFS2 patches for this merge window: 1. Abhi Das contributed a patch to report journal recovery times more accurately during journal replay. 2. Andreas Gruenbacher contributed a patch to fix fallocate chunk size. 3. Andreas added a patch to correctly dirty inodes during rename. 4. Andreas added a patch to improve the comment for function gfs2_block_map. 5. Andreas added a patch to improve kernel trace point iomap end: The physical block address was added. 6. Andreas added a patch to fix a nasty file system corruption bug that surfaced in xfstests 476 in punch-hole/truncate. 7. Andreas fixed a problem Christoph Helwig pointed out, namely, that GFS2 was misusing the IOMAP_ZERO flag. The zeroing of new blocks was moved to the proper fallocate code. 8. I contributed a patch to declare function gfs2_remove_from_ail as static. 9. I added a patch to only set PageChecked for jdata page writes. Abhi Das (1): gfs2: time journal recovery steps accurately Andreas Gruenbacher (6): gfs2: Fix fallocate chunk size gfs2: Dirty source inode during rename gfs2: Improve gfs2_block_map comment gfs2: gfs2_iomap_end tracepoint: log block address gfs2: Check for the end of metadata in punch_hole gfs2: Zero out fallocated blocks in fallocate_chunk Bob Peterson (2): GFS2: Make function gfs2_remove_from_ail static GFS2: Only set PageChecked for jdata pages fs/gfs2/aops.c | 8 fs/gfs2/bmap.c | 38 -- fs/gfs2/dir.c| 13 ++--- fs/gfs2/file.c | 34 +- fs/gfs2/incore.h | 3 --- fs/gfs2/inode.c | 10 +- fs/gfs2/log.c| 2 +- fs/gfs2/log.h| 1 - fs/gfs2/quota.h | 2 ++ fs/gfs2/recovery.c | 20 ++-- fs/gfs2/trace_gfs2.h | 9 +++-- 11 files changed, 68 insertions(+), 72 deletions(-)
GFS2: Please pull patch tagged gfs2-4.16.rc4.fixes
Hi Linus, Would you please pull this additional patch from Andreas Gruenbacher that fixes another unfortunate GFS2 regression? Thanks. Bob Peterson --- The following changes since commit 86f84779d8e92a690b2f281175ea06b884cb6fa4: Merge branch 'siginfo-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace (2018-03-06 12:41:30 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.16.rc4.fixes for you to fetch changes up to 3b5da96e4585a2788da6a07619bda3518d76eb30: gfs2: Fixes to "Implement iomap for block_map" (2) (2018-03-07 11:40:38 -0700) This tag is meant for pulling a patch called gfs2: Fixes to "Implement iomap for block_map" (2). The patch fixes a regression we discovered in commit 49edd5bf42. Andreas Gruenbacher (1): gfs2: Fixes to "Implement iomap for block_map" (2) fs/gfs2/bmap.c | 3 --- 1 file changed, 3 deletions(-)
GFS2: Please pull patch tagged gfs2-4.16.rc4.fixes
Hi Linus, Would you please pull this additional patch from Andreas Gruenbacher that fixes another unfortunate GFS2 regression? Thanks. Bob Peterson --- The following changes since commit 86f84779d8e92a690b2f281175ea06b884cb6fa4: Merge branch 'siginfo-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace (2018-03-06 12:41:30 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.16.rc4.fixes for you to fetch changes up to 3b5da96e4585a2788da6a07619bda3518d76eb30: gfs2: Fixes to "Implement iomap for block_map" (2) (2018-03-07 11:40:38 -0700) This tag is meant for pulling a patch called gfs2: Fixes to "Implement iomap for block_map" (2). The patch fixes a regression we discovered in commit 49edd5bf42. Andreas Gruenbacher (1): gfs2: Fixes to "Implement iomap for block_map" (2) fs/gfs2/bmap.c | 3 --- 1 file changed, 3 deletions(-)
GFS2: Please pull patch tagged gfs2-4.16.rc1.fixes
Hi Linus, Would you please pull this one-off patch from Andreas Gruenbacher that fixes a GFS2 regression? Thanks. Bob Peterson --- The following changes since commit 35277995e17919ab838beae765f440674e8576eb: Merge branch 'x86-pti-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip (2018-02-04 11:45:55 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.16.rc1.fixes for you to fetch changes up to 49edd5bf429c405b3a7f75503845d9f66a47dd4b: gfs2: Fixes to "Implement iomap for block_map" (2018-02-13 13:38:10 -0700) Fix regressions in patch Implement iomap for block_map This tag is meant for pulling a patch called gfs2: Fixes to "Implement iomap for block_map". The patch fixes some regressions we recently discovered in commit 3974320ca6. Andreas Gruenbacher (1): gfs2: Fixes to "Implement iomap for block_map" fs/gfs2/bmap.c | 43 +++ 1 file changed, 23 insertions(+), 20 deletions(-)
GFS2: Please pull patch tagged gfs2-4.16.rc1.fixes
Hi Linus, Would you please pull this one-off patch from Andreas Gruenbacher that fixes a GFS2 regression? Thanks. Bob Peterson --- The following changes since commit 35277995e17919ab838beae765f440674e8576eb: Merge branch 'x86-pti-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip (2018-02-04 11:45:55 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.16.rc1.fixes for you to fetch changes up to 49edd5bf429c405b3a7f75503845d9f66a47dd4b: gfs2: Fixes to "Implement iomap for block_map" (2018-02-13 13:38:10 -0700) Fix regressions in patch Implement iomap for block_map This tag is meant for pulling a patch called gfs2: Fixes to "Implement iomap for block_map". The patch fixes some regressions we recently discovered in commit 3974320ca6. Andreas Gruenbacher (1): gfs2: Fixes to "Implement iomap for block_map" fs/gfs2/bmap.c | 43 +++ 1 file changed, 23 insertions(+), 20 deletions(-)
GFS2: Second Pull request (merge window)
Hi Linus, Sorry to bother you with a second merge request for GFS2, but we found a couple regressions and want to get them fixed in this merge window. Please consider pulling the following changes for the GFS2 file system. Regards, Bob Peterson The following changes since commit 255442c93843f52b6891b21d0b485bf2c97f93c3: Merge tag 'docs-4.16' of git://git.lwn.net/linux (2018-01-31 19:25:25 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.16.fixes2 for you to fetch changes up to 7ac07fdaf840f9b141c6d5c286805107227c0e68: gfs2: Glock dump performance regression fix (2018-02-01 11:27:11 -0700) Andreas Gruenbacher wrote two additional patches that we would like merged in this time. Both are regressions: 1. The first fixes another kernel build dependency problem. 2. The second fixes a performance regression in glock dumps. Andreas Gruenbacher (2): gfs2: Fix the crc32c dependency gfs2: Glock dump performance regression fix fs/gfs2/Kconfig | 3 +-- fs/gfs2/glock.c | 65 ++--- 2 files changed, 44 insertions(+), 24 deletions(-)
GFS2: Second Pull request (merge window)
Hi Linus, Sorry to bother you with a second merge request for GFS2, but we found a couple regressions and want to get them fixed in this merge window. Please consider pulling the following changes for the GFS2 file system. Regards, Bob Peterson The following changes since commit 255442c93843f52b6891b21d0b485bf2c97f93c3: Merge tag 'docs-4.16' of git://git.lwn.net/linux (2018-01-31 19:25:25 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.16.fixes2 for you to fetch changes up to 7ac07fdaf840f9b141c6d5c286805107227c0e68: gfs2: Glock dump performance regression fix (2018-02-01 11:27:11 -0700) Andreas Gruenbacher wrote two additional patches that we would like merged in this time. Both are regressions: 1. The first fixes another kernel build dependency problem. 2. The second fixes a performance regression in glock dumps. Andreas Gruenbacher (2): gfs2: Fix the crc32c dependency gfs2: Glock dump performance regression fix fs/gfs2/Kconfig | 3 +-- fs/gfs2/glock.c | 65 ++--- 2 files changed, 44 insertions(+), 24 deletions(-)
GFS2: Pull request (merge window)
Hi Linus,
Please consider pulling the following changes for the GFS2 file system.
Regards,
Bob Peterson
The following changes since commit 1291a0d5049dbc06baaaf66a9ff3f53db493b19b:
Linux 4.15-rc4 (2017-12-17 18:59:59 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git
tags/gfs2-4.16.fixes
for you to fetch changes up to af38816e4801d2868258085831172a003d1634ff:
gfs2: Add a few missing newlines in messages (2018-01-30 10:32:30 -0700)
We've got 30 patches for this merge window. These generally fall into
five categories: (1) code cleanups, (2) patches related to adding
PUNCH_HOLE support to GFS2, (3) support for new fields in resource group
headers, (4) a few bug fixes, and (5) support for new fields in journal
log headers. These new fields, which were previously unused, are designed
to make it easier to track down file system corruption, and allow fsck.gfs2
to make more intelligent decisions when finding and fixing file system
corruption.
1. Abhi Das contributed a patch to trim the ordered writes list, which
used to grow uncontrollably until unmount.
2. Abhi also added a second patch to trim the ordered writes list.
3. Andreas Gruenbacher removed an unused parameter from function
gfs2_write_jdata_pagevec.
4. Andreas also removed a pointless BUG_ON.
5. Andreas cleaned up an error patch in trunc_start.
6. Andreas removed some unused parameters from truncate.
7. Andreas made gfs2_journaled_truncate more efficient.
8. Andreas cleaned up the support functions for truncate.
9. Andreas fixed metadata read-ahead for truncate to make it faster.
10. Andreas fixed up the non-recursive truncate code.
11. Andreas reworked and renamed function gfs2_block_truncate_page.
12. Andreas generalized the non-recursive truncate code so it can
take a range of values for punch_hole support.
13. Andreas introduced new PUNCH_HOLE support that take advantage
of the previous patches.
14. Andreas contributed a patch to add fallocate with PUNCH_HOLE.
15. Andreas fixed some typos in the comments.
16. Andreas added function gfs2_max_stuffed_size to replace a piece
of code that was needlessly repeated throughout GFS2.
17. Andreas made a minor cleanup to function gfs2_page_add_databufs.
18. Andreas got rid of function gfs2_log_header_in in preparation for
the new log header fields.
19. Andreas also fixed up some missing newlines in kernel messages.
20. Andy Price added a new field to resource groups to indicate where
the next one should be, to allow fsck.gfs2 to make better repairs.
21. Andy also added new rindex fields for consistency checking.
22. Andy added a crc field to resource group headers for consistency
checking.
23. I added a patch to reduce redundancy in functions common to
freeing dinodes.
24. I added a patch to reduce redundancy when writing log headers
between the journalling code and journal recovery code.
25. I added a patch to add new fields to journal log headers based
on a prototype from Steve Whitehouse.
26. I added a patch to log the source of journal log headers so we
can better track down journal corruption.
27. I added a patch to fix a minor comment typo.
28. I also added a patch to fix a BUG in an unlink error path.
29. Steve Whitehouse contributed a patch to fix an incorrect use
of the gfs2_blk2rgrpd function.
30. Tetsuo Handa contributed a patch that fixes incorrect error
handling in function init_gfs2_fs.
Abhi Das (2):
gfs2: Trim the ordered write list in gfs2_ordered_write()
gfs2: Remove inode from ordered write list in gfs2_write_inode()
Andreas Gruenbacher (17):
gfs2: Remove unused gfs2_write_jdata_pagevec parameter
gfs2: Remove pointless BUG_ON
gfs2: Clean up trunc_start error path
gfs2: truncate: Remove unnecessary oldsize parameters
gfs2: Remove minor gfs2_journaled_truncate inefficiencies
gfs2: Clean up {lookup,fillup}_metapath
gfs2: Fix metadata read-ahead during truncate
gfs2: Improve non-recursive delete algorithm
Turn gfs2_block_truncate_page into gfs2_block_zero_range
gfs2: Generalize truncate code
gfs2: Turn trunc_dealloc into punch_hole
gfs2: Implement fallocate(FALLOC_FL_PUNCH_HOLE)
gfs2: Typo fixes
gfs2: Add gfs2_max_stuffed_size
gfs2: Minor gfs2_page_add_databufs cleanup
gfs2: Get rid of gfs2_log_header_in
gfs2: Add a few missing newlines in messages
Andrew Price (3):
gfs2: Add a next-resource-group pointer to resource groups
gfs2: Add rindex fields to rgrp headers
gfs2: Add a crc field to resource group headers
Bob Peterson (7):
GFS2: Combine gfs2_free_di with gfs2_free_uninit_di
GFS2: Reduce code redundancy writing log headers
GFS2: Pull request (merge window)
Hi Linus,
Please consider pulling the following changes for the GFS2 file system.
Regards,
Bob Peterson
The following changes since commit 1291a0d5049dbc06baaaf66a9ff3f53db493b19b:
Linux 4.15-rc4 (2017-12-17 18:59:59 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git
tags/gfs2-4.16.fixes
for you to fetch changes up to af38816e4801d2868258085831172a003d1634ff:
gfs2: Add a few missing newlines in messages (2018-01-30 10:32:30 -0700)
We've got 30 patches for this merge window. These generally fall into
five categories: (1) code cleanups, (2) patches related to adding
PUNCH_HOLE support to GFS2, (3) support for new fields in resource group
headers, (4) a few bug fixes, and (5) support for new fields in journal
log headers. These new fields, which were previously unused, are designed
to make it easier to track down file system corruption, and allow fsck.gfs2
to make more intelligent decisions when finding and fixing file system
corruption.
1. Abhi Das contributed a patch to trim the ordered writes list, which
used to grow uncontrollably until unmount.
2. Abhi also added a second patch to trim the ordered writes list.
3. Andreas Gruenbacher removed an unused parameter from function
gfs2_write_jdata_pagevec.
4. Andreas also removed a pointless BUG_ON.
5. Andreas cleaned up an error patch in trunc_start.
6. Andreas removed some unused parameters from truncate.
7. Andreas made gfs2_journaled_truncate more efficient.
8. Andreas cleaned up the support functions for truncate.
9. Andreas fixed metadata read-ahead for truncate to make it faster.
10. Andreas fixed up the non-recursive truncate code.
11. Andreas reworked and renamed function gfs2_block_truncate_page.
12. Andreas generalized the non-recursive truncate code so it can
take a range of values for punch_hole support.
13. Andreas introduced new PUNCH_HOLE support that take advantage
of the previous patches.
14. Andreas contributed a patch to add fallocate with PUNCH_HOLE.
15. Andreas fixed some typos in the comments.
16. Andreas added function gfs2_max_stuffed_size to replace a piece
of code that was needlessly repeated throughout GFS2.
17. Andreas made a minor cleanup to function gfs2_page_add_databufs.
18. Andreas got rid of function gfs2_log_header_in in preparation for
the new log header fields.
19. Andreas also fixed up some missing newlines in kernel messages.
20. Andy Price added a new field to resource groups to indicate where
the next one should be, to allow fsck.gfs2 to make better repairs.
21. Andy also added new rindex fields for consistency checking.
22. Andy added a crc field to resource group headers for consistency
checking.
23. I added a patch to reduce redundancy in functions common to
freeing dinodes.
24. I added a patch to reduce redundancy when writing log headers
between the journalling code and journal recovery code.
25. I added a patch to add new fields to journal log headers based
on a prototype from Steve Whitehouse.
26. I added a patch to log the source of journal log headers so we
can better track down journal corruption.
27. I added a patch to fix a minor comment typo.
28. I also added a patch to fix a BUG in an unlink error path.
29. Steve Whitehouse contributed a patch to fix an incorrect use
of the gfs2_blk2rgrpd function.
30. Tetsuo Handa contributed a patch that fixes incorrect error
handling in function init_gfs2_fs.
Abhi Das (2):
gfs2: Trim the ordered write list in gfs2_ordered_write()
gfs2: Remove inode from ordered write list in gfs2_write_inode()
Andreas Gruenbacher (17):
gfs2: Remove unused gfs2_write_jdata_pagevec parameter
gfs2: Remove pointless BUG_ON
gfs2: Clean up trunc_start error path
gfs2: truncate: Remove unnecessary oldsize parameters
gfs2: Remove minor gfs2_journaled_truncate inefficiencies
gfs2: Clean up {lookup,fillup}_metapath
gfs2: Fix metadata read-ahead during truncate
gfs2: Improve non-recursive delete algorithm
Turn gfs2_block_truncate_page into gfs2_block_zero_range
gfs2: Generalize truncate code
gfs2: Turn trunc_dealloc into punch_hole
gfs2: Implement fallocate(FALLOC_FL_PUNCH_HOLE)
gfs2: Typo fixes
gfs2: Add gfs2_max_stuffed_size
gfs2: Minor gfs2_page_add_databufs cleanup
gfs2: Get rid of gfs2_log_header_in
gfs2: Add a few missing newlines in messages
Andrew Price (3):
gfs2: Add a next-resource-group pointer to resource groups
gfs2: Add rindex fields to rgrp headers
gfs2: Add a crc field to resource group headers
Bob Peterson (7):
GFS2: Combine gfs2_free_di with gfs2_free_uninit_di
GFS2: Reduce code redundancy writing log headers
Re: linux-next: Signed-off-by missing for commit in the gfs2 tree
- Original Message -
|
|
| On 16/01/18 20:51, Stephen Rothwell wrote:
| > Hi all,
| >
| > Commit
| >
| >7d2040199855 ("gfs2: Add gfs2_blk2rgrpd comment and fix incorrect use")
| >
| > is missing a Signed-off-by from its author.
| >
| Bob, can you add that?
|
| Steve.
This should be fixed now.
Bob
Re: linux-next: Signed-off-by missing for commit in the gfs2 tree
- Original Message -
|
|
| On 16/01/18 20:51, Stephen Rothwell wrote:
| > Hi all,
| >
| > Commit
| >
| >7d2040199855 ("gfs2: Add gfs2_blk2rgrpd comment and fix incorrect use")
| >
| > is missing a Signed-off-by from its author.
| >
| Bob, can you add that?
|
| Steve.
This should be fixed now.
Bob
Re: [Cluster-devel] [PATCH 00/12] drop unneeded newline
- Original Message - | - Original Message - | | Drop newline at the end of a message string when the printing function adds | | a newline. | | Hi Julia, | | NACK. | | As much as it's a pain when searching the source code for output strings, | this patch set goes against the accepted Linux coding style document. See: | | https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings | | Regards, | | Bob Peterson | | Hm. I guess I stand corrected. The document reads: "However, never break user-visible strings such as printk messages, because that breaks the ability to grep for them." Still, the GFS2 and DLM code has a plethora of broken-up printk messages, and I don't like the thought of re-combining them all. Regards, Bob Peterson
Re: [Cluster-devel] [PATCH 00/12] drop unneeded newline
- Original Message - | - Original Message - | | Drop newline at the end of a message string when the printing function adds | | a newline. | | Hi Julia, | | NACK. | | As much as it's a pain when searching the source code for output strings, | this patch set goes against the accepted Linux coding style document. See: | | https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings | | Regards, | | Bob Peterson | | Hm. I guess I stand corrected. The document reads: "However, never break user-visible strings such as printk messages, because that breaks the ability to grep for them." Still, the GFS2 and DLM code has a plethora of broken-up printk messages, and I don't like the thought of re-combining them all. Regards, Bob Peterson
Re: [Cluster-devel] [PATCH 00/12] drop unneeded newline
- Original Message - | Drop newline at the end of a message string when the printing function adds | a newline. Hi Julia, NACK. As much as it's a pain when searching the source code for output strings, this patch set goes against the accepted Linux coding style document. See: https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings Regards, Bob Peterson
Re: [Cluster-devel] [PATCH 00/12] drop unneeded newline
- Original Message - | Drop newline at the end of a message string when the printing function adds | a newline. Hi Julia, NACK. As much as it's a pain when searching the source code for output strings, this patch set goes against the accepted Linux coding style document. See: https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings Regards, Bob Peterson
Re: [PATCH] gfs2: Fix wrong error handling in init_gfs2_fs()
- Original Message - | init_gfs2_fs() is calling e.g. calling unregister_shrinker() without | register_shrinker() when an error occurred during initialization. | Rename goto labels and call appropriate undo function. | | Signed-off-by: Tetsuo Handa <penguin-ker...@i-love.sakura.ne.jp> | --- | fs/gfs2/main.c | 90 | -- | 1 file changed, 44 insertions(+), 46 deletions(-) Hi, ACK. Thanks; this is a nice cleanup. This is now pushed to the for-next branch of the linux-gfs2 tree: https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/?h=for-next=8b0d7f56b97b95a442b6785027a0f80ad1ea54af Regards, Bob Peterson Red Hat File Systems
Re: [PATCH] gfs2: Fix wrong error handling in init_gfs2_fs()
- Original Message - | init_gfs2_fs() is calling e.g. calling unregister_shrinker() without | register_shrinker() when an error occurred during initialization. | Rename goto labels and call appropriate undo function. | | Signed-off-by: Tetsuo Handa | --- | fs/gfs2/main.c | 90 | -- | 1 file changed, 44 insertions(+), 46 deletions(-) Hi, ACK. Thanks; this is a nice cleanup. This is now pushed to the for-next branch of the linux-gfs2 tree: https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/?h=for-next=8b0d7f56b97b95a442b6785027a0f80ad1ea54af Regards, Bob Peterson Red Hat File Systems
GFS2: Pull request (merge window) (redo)
Hi Linus,
I've adjusted this pull request to have a proper diffstat generated by
git merge to a temporary branch, as I did the last time this happened.
So you can pull this one, but probably after Ted's merge request:
https://lkml.org/lkml/2017/11/12/256
Please consider pulling the following changes for the GFS2 file system.
Regards,
Bob Peterson
The following changes since commit 9e66317d3c92ddaab330c125dfe9d06eee268aff:
Linux 4.14-rc3 (2017-10-01 14:54:54 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git
tags/gfs2-4.15.fixes
for you to fetch changes up to d0920a9cd7e735c429c510b523a100db82c937a1:
gfs2: Allow gfs2_xattr_set to be called with the glock held (2017-10-31
14:26:59 +0100)
We've got a total of 17 GFS2 patches for this merge window. The
patches are basically in three categories: (1) patches related to
broken xfstest cases, (2) patches related to improving iomap and
start using it in GFS2, and (3) general typos and clarifications.
Please note that one of the iomap patches extends beyond GFS2 and
affects other file systems, but it was publically reviewed by a
variety of file system people in the community.
1. Andreas has a patch that simply renames variable 'bsize' to 'factor'
to clarify the logic related to gfs2_block_map.
2. He also has a patch to correctly set ctime in the setflags ioctl,
which fixes broken xfstests test 277.
3. He also fixed broken xfstest 258, due to an atime initialization
problem.
4. He also fixed broken xfstest 307, in which GFS2 was not setting
ctime when setting acls.
5. He has a patch to switch general iomap code from blkno to disk
offset for a variety of file systems.
6. He has a patch to add a new IOMAP_F_DATA_INLINE flag for iomap
to indicate blocks that have data mixed with metadata.
7. I contributed a patch to make inode height info part of the
'metapath' data structure to facilitate using iomap in GFS2.
8. I have a patch to start using iomap inside GFS2 and switch GFS2's
block_map functions to use iomap under the covers.
9. I have a patch to switch GFS2's fiemap implementation from using
block_map to using iomap under the covers.
10. Andreas has a patch to implement SEEK_HOLE and SEEK_DATA via
iomap in GFS2.
11. I have a patch related to journaled data pages not being properly
synced to media when writing inodes. This was caught with xfstests.
12. I have a patch to fix another failing xfstest case in which
switching a file from ordered_write to journaled data via set_flags
caused a deadlock.
13. Andreas has a patch to fix failing xfstest case 066, which was
due to not properly syncing dirty inodes when changing extended
attributes.
14. Andreas fixed a minor typo in a comment.
15. Andreas contributed a patch to partially fix xfstest 424, which
involved GET_FLAGS and SET_FLAGS ioctl. This is also a cleanup
and simplification of the translation of flags from fs flags to
gfs2 flags.
16. He also added support for STATX_ATTR_ in statx, which fixed broken
xfstest 424.
17. He also contributed a fix for failing xfstest 093 which fixes a
recursive glock problem with gfs2_xattr_get and _set.
Andreas Gruenbacher (13):
gfs2: Clarify gfs2_block_map
gfs2: Update ctime in setflags ioctl
gfs2: Support negative atimes
gfs2: Always update inode ctime in set_acl
iomap: Switch from blkno to disk offset
iomap: Add IOMAP_F_DATA_INLINE flag
Merge branch 'master' of git://git.kernel.org/.../tytso/ext4
gfs2: Implement SEEK_HOLE / SEEK_DATA via iomap
gfs2: Fix xattr fsync
gfs2: Fix a harmless typo
gfs2: Fix and clean up {GET,SET}FLAGS ioctl
gfs2: Add support for statx inode flags
gfs2: Allow gfs2_xattr_set to be called with the glock held
Bob Peterson (5):
GFS2: Make height info part of metapath
GFS2: Implement iomap for block_map
GFS2: Switch fiemap implementation to use iomap
GFS2: flush the log and all pages for jdata as we do for WB_SYNC_ALL
GFS2: Take inode off order_write list when setting jdata flag
fs/gfs2/Kconfig | 1 +
fs/gfs2/acl.c | 1 +
fs/gfs2/bmap.c| 322
+--
fs/gfs2/bmap.h| 4 +
fs/gfs2/file.c| 124 ++-
fs/gfs2/inode.c | 89 ++--
fs/gfs2/inode.h | 2 +
fs/gfs2/super.c | 5 +-
fs/gfs2/trace_gfs2.h | 65 +++
fs/gfs2/trans.c | 2 +-
fs/gfs2/xattr.c | 63 ++
include/linux/iomap.h | 3 +-
12 files changed, 471 insertions(+), 210 deletions(-)
GFS2: Pull request (merge window) (redo)
Hi Linus,
I've adjusted this pull request to have a proper diffstat generated by
git merge to a temporary branch, as I did the last time this happened.
So you can pull this one, but probably after Ted's merge request:
https://lkml.org/lkml/2017/11/12/256
Please consider pulling the following changes for the GFS2 file system.
Regards,
Bob Peterson
The following changes since commit 9e66317d3c92ddaab330c125dfe9d06eee268aff:
Linux 4.14-rc3 (2017-10-01 14:54:54 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git
tags/gfs2-4.15.fixes
for you to fetch changes up to d0920a9cd7e735c429c510b523a100db82c937a1:
gfs2: Allow gfs2_xattr_set to be called with the glock held (2017-10-31
14:26:59 +0100)
We've got a total of 17 GFS2 patches for this merge window. The
patches are basically in three categories: (1) patches related to
broken xfstest cases, (2) patches related to improving iomap and
start using it in GFS2, and (3) general typos and clarifications.
Please note that one of the iomap patches extends beyond GFS2 and
affects other file systems, but it was publically reviewed by a
variety of file system people in the community.
1. Andreas has a patch that simply renames variable 'bsize' to 'factor'
to clarify the logic related to gfs2_block_map.
2. He also has a patch to correctly set ctime in the setflags ioctl,
which fixes broken xfstests test 277.
3. He also fixed broken xfstest 258, due to an atime initialization
problem.
4. He also fixed broken xfstest 307, in which GFS2 was not setting
ctime when setting acls.
5. He has a patch to switch general iomap code from blkno to disk
offset for a variety of file systems.
6. He has a patch to add a new IOMAP_F_DATA_INLINE flag for iomap
to indicate blocks that have data mixed with metadata.
7. I contributed a patch to make inode height info part of the
'metapath' data structure to facilitate using iomap in GFS2.
8. I have a patch to start using iomap inside GFS2 and switch GFS2's
block_map functions to use iomap under the covers.
9. I have a patch to switch GFS2's fiemap implementation from using
block_map to using iomap under the covers.
10. Andreas has a patch to implement SEEK_HOLE and SEEK_DATA via
iomap in GFS2.
11. I have a patch related to journaled data pages not being properly
synced to media when writing inodes. This was caught with xfstests.
12. I have a patch to fix another failing xfstest case in which
switching a file from ordered_write to journaled data via set_flags
caused a deadlock.
13. Andreas has a patch to fix failing xfstest case 066, which was
due to not properly syncing dirty inodes when changing extended
attributes.
14. Andreas fixed a minor typo in a comment.
15. Andreas contributed a patch to partially fix xfstest 424, which
involved GET_FLAGS and SET_FLAGS ioctl. This is also a cleanup
and simplification of the translation of flags from fs flags to
gfs2 flags.
16. He also added support for STATX_ATTR_ in statx, which fixed broken
xfstest 424.
17. He also contributed a fix for failing xfstest 093 which fixes a
recursive glock problem with gfs2_xattr_get and _set.
Andreas Gruenbacher (13):
gfs2: Clarify gfs2_block_map
gfs2: Update ctime in setflags ioctl
gfs2: Support negative atimes
gfs2: Always update inode ctime in set_acl
iomap: Switch from blkno to disk offset
iomap: Add IOMAP_F_DATA_INLINE flag
Merge branch 'master' of git://git.kernel.org/.../tytso/ext4
gfs2: Implement SEEK_HOLE / SEEK_DATA via iomap
gfs2: Fix xattr fsync
gfs2: Fix a harmless typo
gfs2: Fix and clean up {GET,SET}FLAGS ioctl
gfs2: Add support for statx inode flags
gfs2: Allow gfs2_xattr_set to be called with the glock held
Bob Peterson (5):
GFS2: Make height info part of metapath
GFS2: Implement iomap for block_map
GFS2: Switch fiemap implementation to use iomap
GFS2: flush the log and all pages for jdata as we do for WB_SYNC_ALL
GFS2: Take inode off order_write list when setting jdata flag
fs/gfs2/Kconfig | 1 +
fs/gfs2/acl.c | 1 +
fs/gfs2/bmap.c| 322
+--
fs/gfs2/bmap.h| 4 +
fs/gfs2/file.c| 124 ++-
fs/gfs2/inode.c | 89 ++--
fs/gfs2/inode.h | 2 +
fs/gfs2/super.c | 5 +-
fs/gfs2/trace_gfs2.h | 65 +++
fs/gfs2/trans.c | 2 +-
fs/gfs2/xattr.c | 63 ++
include/linux/iomap.h | 3 +-
12 files changed, 471 insertions(+), 210 deletions(-)
Re: GFS2: Pull request (merge window)
- Original Message - | Hi Linus, | | Please consider pulling the following changes for the GFS2 file system. | | Regards, | | Bob Peterson | | The following changes since commit 9e66317d3c92ddaab330c125dfe9d06eee268aff: | | Linux 4.14-rc3 (2017-10-01 14:54:54 -0700) | | are available in the git repository at: | | git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git | tags/gfs2-4.15.fixes | | for you to fetch changes up to d0920a9cd7e735c429c510b523a100db82c937a1: | | gfs2: Allow gfs2_xattr_set to be called with the glock held (2017-10-31 | 14:26:59 +0100) Hi Linus, I guess I jumped the gun on this. Sorry. This pull request has a dependency on patches that are going through git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4 master due to the iomap changes we need that required another merge branch. So maybe it's best to hold off until those changes get merged. I'll see if I can send a replacement pull request. Regards, Bob Peterson
Re: GFS2: Pull request (merge window)
- Original Message - | Hi Linus, | | Please consider pulling the following changes for the GFS2 file system. | | Regards, | | Bob Peterson | | The following changes since commit 9e66317d3c92ddaab330c125dfe9d06eee268aff: | | Linux 4.14-rc3 (2017-10-01 14:54:54 -0700) | | are available in the git repository at: | | git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git | tags/gfs2-4.15.fixes | | for you to fetch changes up to d0920a9cd7e735c429c510b523a100db82c937a1: | | gfs2: Allow gfs2_xattr_set to be called with the glock held (2017-10-31 | 14:26:59 +0100) Hi Linus, I guess I jumped the gun on this. Sorry. This pull request has a dependency on patches that are going through git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4 master due to the iomap changes we need that required another merge branch. So maybe it's best to hold off until those changes get merged. I'll see if I can send a replacement pull request. Regards, Bob Peterson
GFS2: Pull request (merge window)
Hi Linus,
Please consider pulling the following changes for the GFS2 file system.
Regards,
Bob Peterson
The following changes since commit 9e66317d3c92ddaab330c125dfe9d06eee268aff:
Linux 4.14-rc3 (2017-10-01 14:54:54 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git
tags/gfs2-4.15.fixes
for you to fetch changes up to d0920a9cd7e735c429c510b523a100db82c937a1:
gfs2: Allow gfs2_xattr_set to be called with the glock held (2017-10-31
14:26:59 +0100)
We've got a total of 17 GFS2 patches for this merge window. The
patches are basically in three categories: (1) patches related to
broken xfstest cases, (2) patches related to improving iomap and
start using it in GFS2, and (3) general typos and clarifications.
Please note that one of the iomap patches extends beyond GFS2 and
affects other file systems, but it was publically reviewed by a
variety of file system people in the community.
1. Andreas has a patch that simply renames variable 'bsize' to 'factor'
to clarify the logic related to gfs2_block_map.
2. He also has a patch to correctly set ctime in the setflags ioctl,
which fixes broken xfstests test 277.
3. He also fixed broken xfstest 258, due to an atime initialization
problem.
4. He also fixed broken xfstest 307, in which GFS2 was not setting
ctime when setting acls.
5. He has a patch to switch general iomap code from blkno to disk
offset for a variety of file systems.
6. He has a patch to add a new IOMAP_F_DATA_INLINE flag for iomap
to indicate blocks that have data mixed with metadata.
7. I contributed a patch to make inode height info part of the
'metapath' data structure to facilitate using iomap in GFS2.
8. I have a patch to start using iomap inside GFS2 and switch GFS2's
block_map functions to use iomap under the covers.
9. I have a patch to switch GFS2's fiemap implementation from using
block_map to using iomap under the covers.
10. Andreas has a patch to implement SEEK_HOLE and SEEK_DATA via
iomap in GFS2.
11. I have a patch related to journaled data pages not being properly
synced to media when writing inodes. This was caught with xfstests.
12. I have a patch to fix another failing xfstest case in which
switching a file from ordered_write to journaled data via set_flags
caused a deadlock.
13. Andreas has a patch to fix failing xfstest case 066, which was
due to not properly syncing dirty inodes when changing extended
attributes.
14. Andreas fixed a minor typo in a comment.
15. Andreas contributed a patch to partially fix xfstest 424, which
involved GET_FLAGS and SET_FLAGS ioctl. This is also a cleanup
and simplification of the translation of flags from fs flags to
gfs2 flags.
16. He also added support for STATX_ATTR_ in statx, which fixed broken
xfstest 424.
17. He also contributed a fix for failing xfstest 093 which fixes a
recursive glock problem with gfs2_xattr_get and _set.
Andreas Gruenbacher (13):
gfs2: Clarify gfs2_block_map
gfs2: Update ctime in setflags ioctl
gfs2: Support negative atimes
gfs2: Always update inode ctime in set_acl
iomap: Switch from blkno to disk offset
iomap: Add IOMAP_F_DATA_INLINE flag
Merge branch 'master' of git://git.kernel.org/.../tytso/ext4
gfs2: Implement SEEK_HOLE / SEEK_DATA via iomap
gfs2: Fix xattr fsync
gfs2: Fix a harmless typo
gfs2: Fix and clean up {GET,SET}FLAGS ioctl
gfs2: Add support for statx inode flags
gfs2: Allow gfs2_xattr_set to be called with the glock held
Bob Peterson (5):
GFS2: Make height info part of metapath
GFS2: Implement iomap for block_map
GFS2: Switch fiemap implementation to use iomap
GFS2: flush the log and all pages for jdata as we do for WB_SYNC_ALL
GFS2: Take inode off order_write list when setting jdata flag
fs/buffer.c | 4 +-
fs/dax.c | 2 +-
fs/ext2/inode.c | 4 +-
fs/ext4/inode.c | 4 +-
fs/gfs2/Kconfig | 1 +
fs/gfs2/acl.c | 1 +
fs/gfs2/bmap.c| 322 +-
fs/gfs2/bmap.h| 4 +
fs/gfs2/file.c| 124 ++-
fs/gfs2/inode.c | 89 ++
fs/gfs2/inode.h | 2 +
fs/gfs2/super.c | 5 +-
fs/gfs2/trace_gfs2.h | 65 ++
fs/gfs2/trans.c | 2 +-
fs/gfs2/xattr.c | 63 --
fs/iomap.c| 13 +-
fs/nfsd/blocklayout.c | 4 +-
fs/xfs/xfs_iomap.c| 6 +-
include/linux/iomap.h | 18 +--
19 files changed, 498 insertions(+), 235 deletions(-)
GFS2: Pull request (merge window)
Hi Linus,
Please consider pulling the following changes for the GFS2 file system.
Regards,
Bob Peterson
The following changes since commit 9e66317d3c92ddaab330c125dfe9d06eee268aff:
Linux 4.14-rc3 (2017-10-01 14:54:54 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git
tags/gfs2-4.15.fixes
for you to fetch changes up to d0920a9cd7e735c429c510b523a100db82c937a1:
gfs2: Allow gfs2_xattr_set to be called with the glock held (2017-10-31
14:26:59 +0100)
We've got a total of 17 GFS2 patches for this merge window. The
patches are basically in three categories: (1) patches related to
broken xfstest cases, (2) patches related to improving iomap and
start using it in GFS2, and (3) general typos and clarifications.
Please note that one of the iomap patches extends beyond GFS2 and
affects other file systems, but it was publically reviewed by a
variety of file system people in the community.
1. Andreas has a patch that simply renames variable 'bsize' to 'factor'
to clarify the logic related to gfs2_block_map.
2. He also has a patch to correctly set ctime in the setflags ioctl,
which fixes broken xfstests test 277.
3. He also fixed broken xfstest 258, due to an atime initialization
problem.
4. He also fixed broken xfstest 307, in which GFS2 was not setting
ctime when setting acls.
5. He has a patch to switch general iomap code from blkno to disk
offset for a variety of file systems.
6. He has a patch to add a new IOMAP_F_DATA_INLINE flag for iomap
to indicate blocks that have data mixed with metadata.
7. I contributed a patch to make inode height info part of the
'metapath' data structure to facilitate using iomap in GFS2.
8. I have a patch to start using iomap inside GFS2 and switch GFS2's
block_map functions to use iomap under the covers.
9. I have a patch to switch GFS2's fiemap implementation from using
block_map to using iomap under the covers.
10. Andreas has a patch to implement SEEK_HOLE and SEEK_DATA via
iomap in GFS2.
11. I have a patch related to journaled data pages not being properly
synced to media when writing inodes. This was caught with xfstests.
12. I have a patch to fix another failing xfstest case in which
switching a file from ordered_write to journaled data via set_flags
caused a deadlock.
13. Andreas has a patch to fix failing xfstest case 066, which was
due to not properly syncing dirty inodes when changing extended
attributes.
14. Andreas fixed a minor typo in a comment.
15. Andreas contributed a patch to partially fix xfstest 424, which
involved GET_FLAGS and SET_FLAGS ioctl. This is also a cleanup
and simplification of the translation of flags from fs flags to
gfs2 flags.
16. He also added support for STATX_ATTR_ in statx, which fixed broken
xfstest 424.
17. He also contributed a fix for failing xfstest 093 which fixes a
recursive glock problem with gfs2_xattr_get and _set.
Andreas Gruenbacher (13):
gfs2: Clarify gfs2_block_map
gfs2: Update ctime in setflags ioctl
gfs2: Support negative atimes
gfs2: Always update inode ctime in set_acl
iomap: Switch from blkno to disk offset
iomap: Add IOMAP_F_DATA_INLINE flag
Merge branch 'master' of git://git.kernel.org/.../tytso/ext4
gfs2: Implement SEEK_HOLE / SEEK_DATA via iomap
gfs2: Fix xattr fsync
gfs2: Fix a harmless typo
gfs2: Fix and clean up {GET,SET}FLAGS ioctl
gfs2: Add support for statx inode flags
gfs2: Allow gfs2_xattr_set to be called with the glock held
Bob Peterson (5):
GFS2: Make height info part of metapath
GFS2: Implement iomap for block_map
GFS2: Switch fiemap implementation to use iomap
GFS2: flush the log and all pages for jdata as we do for WB_SYNC_ALL
GFS2: Take inode off order_write list when setting jdata flag
fs/buffer.c | 4 +-
fs/dax.c | 2 +-
fs/ext2/inode.c | 4 +-
fs/ext4/inode.c | 4 +-
fs/gfs2/Kconfig | 1 +
fs/gfs2/acl.c | 1 +
fs/gfs2/bmap.c| 322 +-
fs/gfs2/bmap.h| 4 +
fs/gfs2/file.c| 124 ++-
fs/gfs2/inode.c | 89 ++
fs/gfs2/inode.h | 2 +
fs/gfs2/super.c | 5 +-
fs/gfs2/trace_gfs2.h | 65 ++
fs/gfs2/trans.c | 2 +-
fs/gfs2/xattr.c | 63 --
fs/iomap.c| 13 +-
fs/nfsd/blocklayout.c | 4 +-
fs/xfs/xfs_iomap.c| 6 +-
include/linux/iomap.h | 18 +--
19 files changed, 498 insertions(+), 235 deletions(-)
GFS2: Please pull patch tagged gfs2-for-linus-4.14-rc3
Hi Linus, Would you please pull this one-off patch from Andreas Gruenbacher that fixes a GFS2 regression? Thanks. Bob Peterson --- The following changes since commit 46c1e79fee417f151547aa46fae04ab06cb666f4: Merge branch 'perf-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip (2017-09-13 12:24:20 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-for-linus-4.14-rc3 for you to fetch changes up to 10201655b085df8e000822e496e5d4016a167a36: gfs2: Fix debugfs glocks dump (2017-09-25 12:32:33 -0500) GFS2: Fix an old regression in GFS2's debugfs interface This tag is meant for pulling a patch called "gfs2: Fix debugfs glocks dump" which fixes a regression introduced by commit 88ffbf3e03. The regression caused the glock dump in debugfs to not report all the glocks, which makes debugging extremely difficult. Andreas Gruenbacher (1): gfs2: Fix debugfs glocks dump fs/gfs2/glock.c | 14 +- 1 file changed, 5 insertions(+), 9 deletions(-)
GFS2: Please pull patch tagged gfs2-for-linus-4.14-rc3
Hi Linus, Would you please pull this one-off patch from Andreas Gruenbacher that fixes a GFS2 regression? Thanks. Bob Peterson --- The following changes since commit 46c1e79fee417f151547aa46fae04ab06cb666f4: Merge branch 'perf-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip (2017-09-13 12:24:20 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-for-linus-4.14-rc3 for you to fetch changes up to 10201655b085df8e000822e496e5d4016a167a36: gfs2: Fix debugfs glocks dump (2017-09-25 12:32:33 -0500) GFS2: Fix an old regression in GFS2's debugfs interface This tag is meant for pulling a patch called "gfs2: Fix debugfs glocks dump" which fixes a regression introduced by commit 88ffbf3e03. The regression caused the glock dump in debugfs to not report all the glocks, which makes debugging extremely difficult. Andreas Gruenbacher (1): gfs2: Fix debugfs glocks dump fs/gfs2/glock.c | 14 +- 1 file changed, 5 insertions(+), 9 deletions(-)
GFS2: Pull request (merge window)
Hi Linus, Sorry this is a bit rushed, but I wanted to get this out this week, and I'll be traveling for the next ten days with unknown Internet access. Please consider pulling the following changes for the GFS2 file system. Regards, Bob Peterson The following changes since commit da029c11e6b12f321f36dac8771e833b65cec962: exec: Limit arg stack to at most 75% of _STK_LIM (2017-07-07 20:05:08 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.14.fixes for you to fetch changes up to 309e8cda596f6552a32dd14b969ce9b17f837f2f: gfs2: preserve i_mode if __gfs2_set_acl() fails (2017-08-31 07:53:15 -0500) We've got a whopping 29 GFS2 patches for this merge window, mainly because we held some back from the previous merge window until we could get them perfected and well tested. We have a couple patch sets, including my patch set for protecting glock gl_object and Andreas Gruenbacher's patch set to fix the long-standing shrink- slab hang, plus a bunch of assorted bugs and cleanups: 1. I fixed a bug whereby an IO error would lead to a double-brelse. 2. Andreas Gruenbacher made a minor cleanup to call his relatively new function, gfs2_holder_initialized, rather than doing it manually. This was just missed by a previous patch set. 3. Jan Kara fixed a bug whereby the SGID was being cleared when inheriting ACLs. 4. Andreas found a bug and fixed it in his previous patch, "Get rid of flush_delayed_work in gfs2_evict_inode". A call to flush_delayed_work was deleted from *gfs2_inode_lookup and added to gfs2_create_inode. 5. Wang Xibo found and fixed a list_add call in inode_go_lock that specified the parameters in the wrong order. 6. Coly Li submitted a patch to add the REQ_PRIO to some of GFS2's metadata reads that were accidentally missing them. 7 - 10. I submitted a 4-patch set to protect the glock gl_object field. GFS2 was setting and checking gl_object with no locking mechanism, so the value was occasionally stomped on, which caused file system corruption. 11. I submitted a small cleanup to function gfs2_clear_rgrpd. It was needlessly adding rgrp glocks to the lru list, then pulling them back off immediately. The rgrp glocks don't use the lru list anyway, so doing so was just a waste of time. 12. I submitted a patch that checks the GLOF_LRU flag on a glock before trying to remove it from the lru_list. This avoids a lot of unnecessary spin_lock contention. 13. I submitted a patch to delete GFS2's debugfs files only after we evict all the glocks. Before this patch, GFS2 would delete the debugfs files, and if unmount hung waiting for a glock, there was no way to debug the problem. Now, if a hang occurs during umount, we can examine the debugfs files to figure out why it's hung. 14. Andreas Gruenbacher submitted a patch to fix some trivial typos. 15 - 19. Andreas also submitted a five-part patch set to fix the longstanding hang involving the slab shrinker: dlm requires memory, calls the inode shrinker, which calls gfs2's evict, which calls back into DLM before it can evict an inode. 20. Abhi Das submitted a patch to forcibly flush the active items list to relieve memory pressure. This fixes a long-standing bug whereby GFS2 was getting hung permanently in balance_dirty_pages. 21. Thomas Tai submitted a patch to fix a slab corruption problem due to a residual pointer left in the lock_dlm lockstruct. 22. I submitted a patch to withdraw the file system if IO errors are encountered while writing to the journals or statfs system file which were previously not being sent back up. Before, some IO errors were sometimes not be detected for several hours, and at recovery time, the journal errors made journal replay impossible. 23. Andreas has a patch to fix an annoying format-truncation compiler warning so GFS2 compiles cleanly. 24. I have a patch that fixes a handful of sparse compiler warnings. 25. Andreas fixed up an useless gl_object warning caused by an earlier patch. 26. Arvind Yadav added a patch to properly constify our rhashtable params declare. 27. I added a patch to fix a regression caused by the non-recursive delete and truncate patch that caused file system blocks to not be properly freed. 28. Ernesto A. Fernández added a patch to fix a place where GFS2 would send back the wrong return code setting extended attributes. 29. Ernesto also added a patch to fix a case in which GFS2 was improperly setting an inode's i_mode, potentially granting access to the wrong users. Abhi Das (1): gfs2: forcibly flush ail to relieve memory pressure Andreas Gruenbacher (10): gfs2: Lock holder cleanup (fixup) gfs2: Fix
GFS2: Pull request (merge window)
Hi Linus, Sorry this is a bit rushed, but I wanted to get this out this week, and I'll be traveling for the next ten days with unknown Internet access. Please consider pulling the following changes for the GFS2 file system. Regards, Bob Peterson The following changes since commit da029c11e6b12f321f36dac8771e833b65cec962: exec: Limit arg stack to at most 75% of _STK_LIM (2017-07-07 20:05:08 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.14.fixes for you to fetch changes up to 309e8cda596f6552a32dd14b969ce9b17f837f2f: gfs2: preserve i_mode if __gfs2_set_acl() fails (2017-08-31 07:53:15 -0500) We've got a whopping 29 GFS2 patches for this merge window, mainly because we held some back from the previous merge window until we could get them perfected and well tested. We have a couple patch sets, including my patch set for protecting glock gl_object and Andreas Gruenbacher's patch set to fix the long-standing shrink- slab hang, plus a bunch of assorted bugs and cleanups: 1. I fixed a bug whereby an IO error would lead to a double-brelse. 2. Andreas Gruenbacher made a minor cleanup to call his relatively new function, gfs2_holder_initialized, rather than doing it manually. This was just missed by a previous patch set. 3. Jan Kara fixed a bug whereby the SGID was being cleared when inheriting ACLs. 4. Andreas found a bug and fixed it in his previous patch, "Get rid of flush_delayed_work in gfs2_evict_inode". A call to flush_delayed_work was deleted from *gfs2_inode_lookup and added to gfs2_create_inode. 5. Wang Xibo found and fixed a list_add call in inode_go_lock that specified the parameters in the wrong order. 6. Coly Li submitted a patch to add the REQ_PRIO to some of GFS2's metadata reads that were accidentally missing them. 7 - 10. I submitted a 4-patch set to protect the glock gl_object field. GFS2 was setting and checking gl_object with no locking mechanism, so the value was occasionally stomped on, which caused file system corruption. 11. I submitted a small cleanup to function gfs2_clear_rgrpd. It was needlessly adding rgrp glocks to the lru list, then pulling them back off immediately. The rgrp glocks don't use the lru list anyway, so doing so was just a waste of time. 12. I submitted a patch that checks the GLOF_LRU flag on a glock before trying to remove it from the lru_list. This avoids a lot of unnecessary spin_lock contention. 13. I submitted a patch to delete GFS2's debugfs files only after we evict all the glocks. Before this patch, GFS2 would delete the debugfs files, and if unmount hung waiting for a glock, there was no way to debug the problem. Now, if a hang occurs during umount, we can examine the debugfs files to figure out why it's hung. 14. Andreas Gruenbacher submitted a patch to fix some trivial typos. 15 - 19. Andreas also submitted a five-part patch set to fix the longstanding hang involving the slab shrinker: dlm requires memory, calls the inode shrinker, which calls gfs2's evict, which calls back into DLM before it can evict an inode. 20. Abhi Das submitted a patch to forcibly flush the active items list to relieve memory pressure. This fixes a long-standing bug whereby GFS2 was getting hung permanently in balance_dirty_pages. 21. Thomas Tai submitted a patch to fix a slab corruption problem due to a residual pointer left in the lock_dlm lockstruct. 22. I submitted a patch to withdraw the file system if IO errors are encountered while writing to the journals or statfs system file which were previously not being sent back up. Before, some IO errors were sometimes not be detected for several hours, and at recovery time, the journal errors made journal replay impossible. 23. Andreas has a patch to fix an annoying format-truncation compiler warning so GFS2 compiles cleanly. 24. I have a patch that fixes a handful of sparse compiler warnings. 25. Andreas fixed up an useless gl_object warning caused by an earlier patch. 26. Arvind Yadav added a patch to properly constify our rhashtable params declare. 27. I added a patch to fix a regression caused by the non-recursive delete and truncate patch that caused file system blocks to not be properly freed. 28. Ernesto A. Fernández added a patch to fix a place where GFS2 would send back the wrong return code setting extended attributes. 29. Ernesto also added a patch to fix a case in which GFS2 was improperly setting an inode's i_mode, potentially granting access to the wrong users. Abhi Das (1): gfs2: forcibly flush ail to relieve memory pressure Andreas Gruenbacher (10): gfs2: Lock holder cleanup (fixup) gfs2: Fix
Re: [PATCH] gfs2: constify rhashtable_params
- Original Message - | rhashtable_params are not supposed to change at runtime. All | Functions rhashtable_* working with const rhashtable_params | provided by . So mark the non-const structs | as const. | | Signed-off-by: Arvind Yadav <arvind.yadav...@gmail.com> | --- Hi, Thanks. This is now pushed to the for-next branch of the linux-gfs2 tree: https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/fs/gfs2?h=for-next=d296b15ed58231bd991c0fb0f3592d595539bcd1 Regards, Bob Peterson Red Hat File Systems
Re: [PATCH] gfs2: constify rhashtable_params
- Original Message - | rhashtable_params are not supposed to change at runtime. All | Functions rhashtable_* working with const rhashtable_params | provided by . So mark the non-const structs | as const. | | Signed-off-by: Arvind Yadav | --- Hi, Thanks. This is now pushed to the for-next branch of the linux-gfs2 tree: https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/fs/gfs2?h=for-next=d296b15ed58231bd991c0fb0f3592d595539bcd1 Regards, Bob Peterson Red Hat File Systems
[PATCH] tipc: Fix tipc_sk_reinit handling of -EAGAIN
Hi,
In 9dbbfb0ab6680c6a85609041011484e6658e7d3c function tipc_sk_reinit
had additional logic added to loop in the event that function
rhashtable_walk_next() returned -EAGAIN. No worries.
However, if rhashtable_walk_start returns -EAGAIN, it does "continue",
and therefore skips the call to rhashtable_walk_stop(). That has
the effect of calling rcu_read_lock() without its paired call to
rcu_read_unlock(). Since rcu_read_lock() may be nested, the problem
may not be apparent for a while, especially since resize events may
be rare. But the comments to rhashtable_walk_start() state:
* ...Note that we take the RCU lock in all
* cases including when we return an error. So you must always call
* rhashtable_walk_stop to clean up.
This patch replaces the continue with a goto and label to ensure a
matching call to rhashtable_walk_stop().
Signed-off-by: Bob Peterson <rpete...@redhat.com>
---
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 101e3597338f..d50edd6e0019 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -2255,8 +2255,8 @@ void tipc_sk_reinit(struct net *net)
do {
tsk = ERR_PTR(rhashtable_walk_start());
- if (tsk)
- continue;
+ if (IS_ERR(tsk))
+ goto walk_stop;
while ((tsk = rhashtable_walk_next()) && !IS_ERR(tsk)) {
spin_lock_bh(>sk.sk_lock.slock);
@@ -2265,7 +2265,7 @@ void tipc_sk_reinit(struct net *net)
msg_set_orignode(msg, tn->own_addr);
spin_unlock_bh(>sk.sk_lock.slock);
}
-
+walk_stop:
rhashtable_walk_stop();
} while (tsk == ERR_PTR(-EAGAIN));
}
[PATCH] tipc: Fix tipc_sk_reinit handling of -EAGAIN
Hi,
In 9dbbfb0ab6680c6a85609041011484e6658e7d3c function tipc_sk_reinit
had additional logic added to loop in the event that function
rhashtable_walk_next() returned -EAGAIN. No worries.
However, if rhashtable_walk_start returns -EAGAIN, it does "continue",
and therefore skips the call to rhashtable_walk_stop(). That has
the effect of calling rcu_read_lock() without its paired call to
rcu_read_unlock(). Since rcu_read_lock() may be nested, the problem
may not be apparent for a while, especially since resize events may
be rare. But the comments to rhashtable_walk_start() state:
* ...Note that we take the RCU lock in all
* cases including when we return an error. So you must always call
* rhashtable_walk_stop to clean up.
This patch replaces the continue with a goto and label to ensure a
matching call to rhashtable_walk_stop().
Signed-off-by: Bob Peterson
---
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 101e3597338f..d50edd6e0019 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -2255,8 +2255,8 @@ void tipc_sk_reinit(struct net *net)
do {
tsk = ERR_PTR(rhashtable_walk_start());
- if (tsk)
- continue;
+ if (IS_ERR(tsk))
+ goto walk_stop;
while ((tsk = rhashtable_walk_next()) && !IS_ERR(tsk)) {
spin_lock_bh(>sk.sk_lock.slock);
@@ -2265,7 +2265,7 @@ void tipc_sk_reinit(struct net *net)
msg_set_orignode(msg, tn->own_addr);
spin_unlock_bh(>sk.sk_lock.slock);
}
-
+walk_stop:
rhashtable_walk_stop();
} while (tsk == ERR_PTR(-EAGAIN));
}
Re: [PATCH v2 2/4] mm: add file_fdatawait_range and file_write_and_wait
- Original Message - | > If this can be called from anywhere without fs locks, then i_size is not | > known. That has been a problem in the past since i_size may have changed | > on another node. We avoid that in this case due to only changing i_size | > under an exclusive lock, and also only having dirty pages when we have | > an exclusive lock. There is another case though, if the inode is a block | > device, i_size will be zero. That is the case for the address space that | > looks after rgrps for GFS2. We do (luckily!) call | > filemap_fdatawait_range() directly in that case. For "normal" inodes | > though, the address space for metadata is backed by the block device | > inode, so that looks like it might be an issue, since | > fs/gfs2/glops.c:inode_go_sync() calls filemap_fdatawait() on the | > metamapping. It might potentially be an issue in other cases too, | > | > Steve. | > | | Some of those do sound problematic. | | Again though, we're only waiting on writeback here, and I assume with | gfs2 that would only be pages that were written on the local node. | | Is it possible to have pages under writeback and in still in the tree, | but that are beyond the current i_size? It seems like that's the main | worrisome case. | | -- | Jeff Layton <jlay...@redhat.com> Hi Jeff, I believe the answer is yes. I was recently "bitten" by a case where (whether due to a bug or not) I had blocks allocated in a GFS2 file beyond i_size. I had implemented a delete algorithm that used i_size, but I found cases where files couldn't be deleted because of blocks hanging out past EOF. I'm not sure if they can be in writeback, but possibly. It's already on my "to investigate" list, but I haven't gotten to it yet. Yes, it seems like a bug. Yes, we need to fix it. But now there may be lots of legacy file systems out in the field that have this problem. Not sure if they can get to writeback until I study the situation more closely. I believe Ben Marzinski also may have come across a case in which we can have blocks in writeback that are beyond i_size. See the commit message on Ben's patch here: https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/fs/gfs2?h=for-next=fd4c5748b8d3f7420e8932ed0bde3d53cc8acc9d Regards, Bob Peterson Red Hat File Systems
Re: [PATCH v2 2/4] mm: add file_fdatawait_range and file_write_and_wait
- Original Message - | > If this can be called from anywhere without fs locks, then i_size is not | > known. That has been a problem in the past since i_size may have changed | > on another node. We avoid that in this case due to only changing i_size | > under an exclusive lock, and also only having dirty pages when we have | > an exclusive lock. There is another case though, if the inode is a block | > device, i_size will be zero. That is the case for the address space that | > looks after rgrps for GFS2. We do (luckily!) call | > filemap_fdatawait_range() directly in that case. For "normal" inodes | > though, the address space for metadata is backed by the block device | > inode, so that looks like it might be an issue, since | > fs/gfs2/glops.c:inode_go_sync() calls filemap_fdatawait() on the | > metamapping. It might potentially be an issue in other cases too, | > | > Steve. | > | | Some of those do sound problematic. | | Again though, we're only waiting on writeback here, and I assume with | gfs2 that would only be pages that were written on the local node. | | Is it possible to have pages under writeback and in still in the tree, | but that are beyond the current i_size? It seems like that's the main | worrisome case. | | -- | Jeff Layton Hi Jeff, I believe the answer is yes. I was recently "bitten" by a case where (whether due to a bug or not) I had blocks allocated in a GFS2 file beyond i_size. I had implemented a delete algorithm that used i_size, but I found cases where files couldn't be deleted because of blocks hanging out past EOF. I'm not sure if they can be in writeback, but possibly. It's already on my "to investigate" list, but I haven't gotten to it yet. Yes, it seems like a bug. Yes, we need to fix it. But now there may be lots of legacy file systems out in the field that have this problem. Not sure if they can get to writeback until I study the situation more closely. I believe Ben Marzinski also may have come across a case in which we can have blocks in writeback that are beyond i_size. See the commit message on Ben's patch here: https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/fs/gfs2?h=for-next=fd4c5748b8d3f7420e8932ed0bde3d53cc8acc9d Regards, Bob Peterson Red Hat File Systems
Re: [PATCH v2 4/4] gfs2: convert to errseq_t based writeback error reporting for fsync
- Original Message - | On Wed, 2017-07-26 at 12:21 -0700, Matthew Wilcox wrote: | > On Wed, Jul 26, 2017 at 01:55:38PM -0400, Jeff Layton wrote: | > > @@ -668,12 +668,14 @@ static int gfs2_fsync(struct file *file, loff_t | > > start, loff_t end, | > > if (ret) | > > return ret; | > > if (gfs2_is_jdata(ip)) | > > - filemap_write_and_wait(mapping); | > > + ret = file_write_and_wait(file); | > > + if (ret) | > > + return ret; | > > gfs2_ail_flush(ip->i_gl, 1); | > > } | > | > Do we want to skip flushing the AIL if there was an error (possibly | > previously encountered)? I'd think we'd want to flush the AIL then report | > the error, like this: | > | | I wondered about that. Note that earlier in the function, we also bail | out without flushing the AIL if sync_inode_metadata fails, so I assumed | that we'd want to do the same here. | | I could definitely be wrong and am fine with changing it if so. | Discarding the error like we do today seems wrong though. | | Bob, thoughts? Hi Jeff, Matthew, I'm not sure there's a right or wrong answer here. I don't know what's best from a "correctness" point of view. I guess I'm leaning toward Jeff's original solution where we don't call gfs2_ail_flush() on error. The main purpose of ail_flush is to go through buffer descriptors (bds) attached to the glock and generate revokes for them in a new transaction. If there's an error condition, trying to go through more hoops will probably just get us into more trouble. If the error is -ENOMEM, we don't want to allocate new memory for the new transaction. If the error is -EIO, we probably don't want to encourage more writing either. So on the one hand, it might be good to get rid of the buffer descriptors so we don't leak memory, but that's probably also done elsewhere. I have not chased down what happens in that case, but the same thing would happen in the existing -EIO case a few lines above. On the other hand, we probably don't want to start a new transaction and start adding revokes to it, and such, due to the error. Perhaps Steve Whitehouse can weigh in? Regards, Bob Peterson Red Hat File Systems
Re: [PATCH v2 4/4] gfs2: convert to errseq_t based writeback error reporting for fsync
- Original Message - | On Wed, 2017-07-26 at 12:21 -0700, Matthew Wilcox wrote: | > On Wed, Jul 26, 2017 at 01:55:38PM -0400, Jeff Layton wrote: | > > @@ -668,12 +668,14 @@ static int gfs2_fsync(struct file *file, loff_t | > > start, loff_t end, | > > if (ret) | > > return ret; | > > if (gfs2_is_jdata(ip)) | > > - filemap_write_and_wait(mapping); | > > + ret = file_write_and_wait(file); | > > + if (ret) | > > + return ret; | > > gfs2_ail_flush(ip->i_gl, 1); | > > } | > | > Do we want to skip flushing the AIL if there was an error (possibly | > previously encountered)? I'd think we'd want to flush the AIL then report | > the error, like this: | > | | I wondered about that. Note that earlier in the function, we also bail | out without flushing the AIL if sync_inode_metadata fails, so I assumed | that we'd want to do the same here. | | I could definitely be wrong and am fine with changing it if so. | Discarding the error like we do today seems wrong though. | | Bob, thoughts? Hi Jeff, Matthew, I'm not sure there's a right or wrong answer here. I don't know what's best from a "correctness" point of view. I guess I'm leaning toward Jeff's original solution where we don't call gfs2_ail_flush() on error. The main purpose of ail_flush is to go through buffer descriptors (bds) attached to the glock and generate revokes for them in a new transaction. If there's an error condition, trying to go through more hoops will probably just get us into more trouble. If the error is -ENOMEM, we don't want to allocate new memory for the new transaction. If the error is -EIO, we probably don't want to encourage more writing either. So on the one hand, it might be good to get rid of the buffer descriptors so we don't leak memory, but that's probably also done elsewhere. I have not chased down what happens in that case, but the same thing would happen in the existing -EIO case a few lines above. On the other hand, we probably don't want to start a new transaction and start adding revokes to it, and such, due to the error. Perhaps Steve Whitehouse can weigh in? Regards, Bob Peterson Red Hat File Systems
Re: [PATCH v2 2/4] mm: add file_fdatawait_range and file_write_and_wait
- Original Message -
| From: Jeff Layton <jlay...@redhat.com>
|
| Some filesystem fsync routines will need these.
|
| Signed-off-by: Jeff Layton <jlay...@redhat.com>
| ---
| include/linux/fs.h | 7 ++-
| mm/filemap.c | 56
| ++
| 2 files changed, 62 insertions(+), 1 deletion(-)
(snip)
| diff --git a/mm/filemap.c b/mm/filemap.c
| index 72e46e6f0d9a..b904a8dfa43d 100644
| --- a/mm/filemap.c
| +++ b/mm/filemap.c
(snip)
| @@ -675,6 +698,39 @@ int file_write_and_wait_range(struct file *file, loff_t
| lstart, loff_t lend)
| EXPORT_SYMBOL(file_write_and_wait_range);
|
| /**
| + * file_write_and_wait - write out whole file and wait on it and return any
| + *writeback errors since we last checked
| + * @file: file to write back and wait on
| + *
| + * Write back the whole file and wait on its mapping. Afterward, check for
| + * errors that may have occurred since our file->f_wb_err cursor was last
| + * updated.
| + */
| +int file_write_and_wait(struct file *file)
| +{
| + int err = 0, err2;
| + struct address_space *mapping = file->f_mapping;
| +
| + if ((!dax_mapping(mapping) && mapping->nrpages) ||
| + (dax_mapping(mapping) && mapping->nrexceptional)) {
Seems like we should make the new function mapping_needs_writeback more
central (mm.h or fs.h?) and call it here ^.
| + err = filemap_fdatawrite(mapping);
| + /* See comment of filemap_write_and_wait() */
| + if (err != -EIO) {
| + loff_t i_size = i_size_read(mapping->host);
| +
| + if (i_size != 0)
| + __filemap_fdatawait_range(mapping, 0,
| + i_size - 1);
| + }
| + }
| + err2 = file_check_and_advance_wb_err(file);
| + if (!err)
| + err = err2;
| + return err;
In the past, I've seen more elegant constructs like:
return (err ? err : err2);
but I don't know what's considered more ugly or hackish.
Regards,
Bob Peterson
Red Hat File Systems
Re: [PATCH v2 2/4] mm: add file_fdatawait_range and file_write_and_wait
- Original Message -
| From: Jeff Layton
|
| Some filesystem fsync routines will need these.
|
| Signed-off-by: Jeff Layton
| ---
| include/linux/fs.h | 7 ++-
| mm/filemap.c | 56
| ++
| 2 files changed, 62 insertions(+), 1 deletion(-)
(snip)
| diff --git a/mm/filemap.c b/mm/filemap.c
| index 72e46e6f0d9a..b904a8dfa43d 100644
| --- a/mm/filemap.c
| +++ b/mm/filemap.c
(snip)
| @@ -675,6 +698,39 @@ int file_write_and_wait_range(struct file *file, loff_t
| lstart, loff_t lend)
| EXPORT_SYMBOL(file_write_and_wait_range);
|
| /**
| + * file_write_and_wait - write out whole file and wait on it and return any
| + *writeback errors since we last checked
| + * @file: file to write back and wait on
| + *
| + * Write back the whole file and wait on its mapping. Afterward, check for
| + * errors that may have occurred since our file->f_wb_err cursor was last
| + * updated.
| + */
| +int file_write_and_wait(struct file *file)
| +{
| + int err = 0, err2;
| + struct address_space *mapping = file->f_mapping;
| +
| + if ((!dax_mapping(mapping) && mapping->nrpages) ||
| + (dax_mapping(mapping) && mapping->nrexceptional)) {
Seems like we should make the new function mapping_needs_writeback more
central (mm.h or fs.h?) and call it here ^.
| + err = filemap_fdatawrite(mapping);
| + /* See comment of filemap_write_and_wait() */
| + if (err != -EIO) {
| + loff_t i_size = i_size_read(mapping->host);
| +
| + if (i_size != 0)
| + __filemap_fdatawait_range(mapping, 0,
| + i_size - 1);
| + }
| + }
| + err2 = file_check_and_advance_wb_err(file);
| + if (!err)
| + err = err2;
| + return err;
In the past, I've seen more elegant constructs like:
return (err ? err : err2);
but I don't know what's considered more ugly or hackish.
Regards,
Bob Peterson
Red Hat File Systems
Re: [PATCH] GFS2: fix code parameter error in inode_go_lock
- Original Message -
| In inode_go_lock() function, the parameter order of list_add() is error.
| According to the define of list_add(), the first parameter is new entry
| and the second is the list head, so ip->i_trunc_list should be the
| first parameter and the sdp->sd_trunc_list should be second.
|
| Signed-off-by: Wang Xibo<wang.x...@zte.com.cn>
| Signed-off-by: Xiao Likun<xiao.li...@zte.com.cn>
| ---
| fs/gfs2/glops.c | 2 +-
| 1 file changed, 1 insertion(+), 1 deletion(-)
|
| diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
| index 5e69636..28c203a 100644
| --- a/fs/gfs2/glops.c
| +++ b/fs/gfs2/glops.c
| @@ -470,7 +470,7 @@ static int inode_go_lock(struct gfs2_holder *gh)
| (gh->gh_state == LM_ST_EXCLUSIVE)) {
| spin_lock(>sd_trunc_lock);
| if (list_empty(>i_trunc_list))
| - list_add(>sd_trunc_list, >i_trunc_list);
| + list_add(>i_trunc_list, >sd_trunc_list);
| spin_unlock(>sd_trunc_lock);
| wake_up(>sd_quota_wait);
| return 1;
| --
| 1.8.3.1
|
|
|
Hi,
Good catch! This is now applied to the for-next branch of the linux-gfs2 tree:
https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/fs/gfs2?h=for-next=e7cb550d79fba5876616ed33ccafafc4e2bd7e2e
Regards,
Bob Peterson
Red Hat File Systems
Re: [PATCH] GFS2: fix code parameter error in inode_go_lock
- Original Message -
| In inode_go_lock() function, the parameter order of list_add() is error.
| According to the define of list_add(), the first parameter is new entry
| and the second is the list head, so ip->i_trunc_list should be the
| first parameter and the sdp->sd_trunc_list should be second.
|
| Signed-off-by: Wang Xibo
| Signed-off-by: Xiao Likun
| ---
| fs/gfs2/glops.c | 2 +-
| 1 file changed, 1 insertion(+), 1 deletion(-)
|
| diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
| index 5e69636..28c203a 100644
| --- a/fs/gfs2/glops.c
| +++ b/fs/gfs2/glops.c
| @@ -470,7 +470,7 @@ static int inode_go_lock(struct gfs2_holder *gh)
| (gh->gh_state == LM_ST_EXCLUSIVE)) {
| spin_lock(>sd_trunc_lock);
| if (list_empty(>i_trunc_list))
| - list_add(>sd_trunc_list, >i_trunc_list);
| + list_add(>i_trunc_list, >sd_trunc_list);
| spin_unlock(>sd_trunc_lock);
| wake_up(>sd_quota_wait);
| return 1;
| --
| 1.8.3.1
|
|
|
Hi,
Good catch! This is now applied to the for-next branch of the linux-gfs2 tree:
https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/fs/gfs2?h=for-next=e7cb550d79fba5876616ed33ccafafc4e2bd7e2e
Regards,
Bob Peterson
Red Hat File Systems
GFS2: Pull request (merge window) addendum
Hi Linus, Sorry for the additional merge request, but Andreas discovered this problem soon after you processed our last gfs2 merge. This fixes a regression introduced by a patch we did in mid-2015, so best to get it fixed. It just re-adds code that had been there originally. Please consider pulling the following additional patch for the GFS2 file system. Regards, Bob Peterson The following changes since commit 9b51f04424e17051a89ab32d892ca66b2a104825: Merge branch 'parisc-4.13-2' of git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux (2017-07-05 17:41:31 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.13.fixes.addendum for you to fetch changes up to 961ae1d83d055a4b9ebbfb4cc8ca62ec1a7a3b74: gfs2: Fix glock rhashtable rcu bug (2017-07-07 13:22:05 -0500) This is a high-priority addendum patch for a regression introduced by commit 88ffbf3e03. Some code was reverted that should not have been. This patch from Andreas Gruenbacher adds it back in. gfs2: Fix glock rhashtable rcu bug Andreas Gruenbacher (1): gfs2: Fix glock rhashtable rcu bug fs/gfs2/glock.c | 11 +-- fs/gfs2/incore.h | 1 + 2 files changed, 10 insertions(+), 2 deletions(-)
GFS2: Pull request (merge window) addendum
Hi Linus, Sorry for the additional merge request, but Andreas discovered this problem soon after you processed our last gfs2 merge. This fixes a regression introduced by a patch we did in mid-2015, so best to get it fixed. It just re-adds code that had been there originally. Please consider pulling the following additional patch for the GFS2 file system. Regards, Bob Peterson The following changes since commit 9b51f04424e17051a89ab32d892ca66b2a104825: Merge branch 'parisc-4.13-2' of git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux (2017-07-05 17:41:31 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.13.fixes.addendum for you to fetch changes up to 961ae1d83d055a4b9ebbfb4cc8ca62ec1a7a3b74: gfs2: Fix glock rhashtable rcu bug (2017-07-07 13:22:05 -0500) This is a high-priority addendum patch for a regression introduced by commit 88ffbf3e03. Some code was reverted that should not have been. This patch from Andreas Gruenbacher adds it back in. gfs2: Fix glock rhashtable rcu bug Andreas Gruenbacher (1): gfs2: Fix glock rhashtable rcu bug fs/gfs2/glock.c | 11 +-- fs/gfs2/incore.h | 1 + 2 files changed, 10 insertions(+), 2 deletions(-)
GFS2: Pull request (merge window)
Hi Linus, Please consider pulling the following changes for the GFS2 file system. Regards, Bob Peterson The following changes since commit 3ef2bc099d1cce09e2844467e2ced98e1a44609d: Merge tag 'devicetree-for-4.12' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux (2017-05-05 19:33:07 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.13.fixes for you to fetch changes up to 29695254ec51506d97b2a90ac27baabb438c70dd: GFS2: constify attribute_group structures. (2017-07-05 07:21:14 -0500) We've got eight GFS2 patches for this merge window: 1. Andreas Gruenbacher has four patches related to cleaning up the GFS2 inode evict process. This is about half of his patches designed to fix a long-standing GFS2 hang related to the inode shrinker. (Shrinker calls gfs2 evict, evict calls DLM, DLM requires memory and blocks on the shrinker.) These 4 patches have been well tested. His second set of patches are still being tested, so I plan to hold them until the next merge window, after we have more weeks of testing. The first patch eliminates the flush_delayed_work, which can block. 2. Andreas's second patch protects setting of gl_object for rgrps with a spin_lock to prevent proven races. 3. His third patch introduces a centralized mechanism for queueing glock work with better reference counting, to prevent more races. 4. His fourth patch retains a reference to inode glocks when an error occurs while creating an inode. This keeps the subsequent evict from needing to reacquire the glock, which might call into DLM and block in low memory conditions. 5. Arvind Yadav has a patch to add const to attribute_group structures. 6. I have a patch to detect directory entry inconsistencies and withdraw the file system if any are found. Better that than silent corruption. 7. I have a patch to remove a vestigial variable from glock structures, saving some slab space. 8. I have another patch to remove a vestigial variable from the GFS2 in-core superblock structure. Andreas Gruenbacher (4): gfs2: Get rid of flush_delayed_work in gfs2_evict_inode gfs2: Protect gl->gl_object by spin lock gfs2: Clean up glock work enqueuing gfs2: gfs2_create_inode: Keep glock across iput Arvind Yadav (1): GFS2: constify attribute_group structures. Bob Peterson (3): GFS2: Withdraw when directory entry inconsistencies are detected GFS2: Remove gl_list from glock structure GFS2: Eliminate vestigial sd_log_flush_wrapped fs/gfs2/bmap.c | 2 +- fs/gfs2/dir.c| 7 ++-- fs/gfs2/glock.c | 124 +-- fs/gfs2/glock.h | 7 fs/gfs2/glops.c | 56 - fs/gfs2/incore.h | 5 ++- fs/gfs2/inode.c | 19 + fs/gfs2/log.c| 3 -- fs/gfs2/lops.c | 6 +-- fs/gfs2/main.c | 1 - fs/gfs2/rgrp.c | 6 +-- fs/gfs2/super.c | 33 +-- fs/gfs2/sys.c| 4 +- fs/gfs2/xattr.c | 4 +- 14 files changed, 175 insertions(+), 102 deletions(-)
GFS2: Pull request (merge window)
Hi Linus, Please consider pulling the following changes for the GFS2 file system. Regards, Bob Peterson The following changes since commit 3ef2bc099d1cce09e2844467e2ced98e1a44609d: Merge tag 'devicetree-for-4.12' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux (2017-05-05 19:33:07 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.13.fixes for you to fetch changes up to 29695254ec51506d97b2a90ac27baabb438c70dd: GFS2: constify attribute_group structures. (2017-07-05 07:21:14 -0500) We've got eight GFS2 patches for this merge window: 1. Andreas Gruenbacher has four patches related to cleaning up the GFS2 inode evict process. This is about half of his patches designed to fix a long-standing GFS2 hang related to the inode shrinker. (Shrinker calls gfs2 evict, evict calls DLM, DLM requires memory and blocks on the shrinker.) These 4 patches have been well tested. His second set of patches are still being tested, so I plan to hold them until the next merge window, after we have more weeks of testing. The first patch eliminates the flush_delayed_work, which can block. 2. Andreas's second patch protects setting of gl_object for rgrps with a spin_lock to prevent proven races. 3. His third patch introduces a centralized mechanism for queueing glock work with better reference counting, to prevent more races. 4. His fourth patch retains a reference to inode glocks when an error occurs while creating an inode. This keeps the subsequent evict from needing to reacquire the glock, which might call into DLM and block in low memory conditions. 5. Arvind Yadav has a patch to add const to attribute_group structures. 6. I have a patch to detect directory entry inconsistencies and withdraw the file system if any are found. Better that than silent corruption. 7. I have a patch to remove a vestigial variable from glock structures, saving some slab space. 8. I have another patch to remove a vestigial variable from the GFS2 in-core superblock structure. Andreas Gruenbacher (4): gfs2: Get rid of flush_delayed_work in gfs2_evict_inode gfs2: Protect gl->gl_object by spin lock gfs2: Clean up glock work enqueuing gfs2: gfs2_create_inode: Keep glock across iput Arvind Yadav (1): GFS2: constify attribute_group structures. Bob Peterson (3): GFS2: Withdraw when directory entry inconsistencies are detected GFS2: Remove gl_list from glock structure GFS2: Eliminate vestigial sd_log_flush_wrapped fs/gfs2/bmap.c | 2 +- fs/gfs2/dir.c| 7 ++-- fs/gfs2/glock.c | 124 +-- fs/gfs2/glock.h | 7 fs/gfs2/glops.c | 56 - fs/gfs2/incore.h | 5 ++- fs/gfs2/inode.c | 19 + fs/gfs2/log.c| 3 -- fs/gfs2/lops.c | 6 +-- fs/gfs2/main.c | 1 - fs/gfs2/rgrp.c | 6 +-- fs/gfs2/super.c | 33 +-- fs/gfs2/sys.c| 4 +- fs/gfs2/xattr.c | 4 +- 14 files changed, 175 insertions(+), 102 deletions(-)
Re: [PATCH] fs: GFS2: constify attribute_group structures.
- Original Message - | attribute_groups are not supposed to change at runtime. All functions | working with attribute_groups provided by work with const | attribute_group. So mark the non-const structs as const. | | File size before: |text data bss dec hex filename |5259 1344 8661119d3 fs/gfs2/sys.o | | File size After adding 'const': |text data bss dec hex filename |5371 1216 8659519c3 fs/gfs2/sys.o | | Signed-off-by: Arvind Yadav <arvind.yadav...@gmail.com> Hi, Thanks. This is now applied to the for-next branch of the linux-gfs2 tree: 1fc2e87 GFS2: constify attribute_group structures. Regards, Bob Peterson Red Hat File Systems
Re: [PATCH] fs: GFS2: constify attribute_group structures.
- Original Message - | attribute_groups are not supposed to change at runtime. All functions | working with attribute_groups provided by work with const | attribute_group. So mark the non-const structs as const. | | File size before: |text data bss dec hex filename |5259 1344 8661119d3 fs/gfs2/sys.o | | File size After adding 'const': |text data bss dec hex filename |5371 1216 8659519c3 fs/gfs2/sys.o | | Signed-off-by: Arvind Yadav Hi, Thanks. This is now applied to the for-next branch of the linux-gfs2 tree: 1fc2e87 GFS2: constify attribute_group structures. Regards, Bob Peterson Red Hat File Systems
Re: [PATCH v4 23/27] gfs2: clean up some filemap_* calls
- Original Message - | In some places, it's trying to reset the mapping error after calling | filemap_fdatawait. That's no longer required. Also, turn several | filemap_fdatawrite+filemap_fdatawait calls into filemap_write_and_wait. | That will at least return writeback errors that occur during the write | phase. | | Signed-off-by: Jeff Layton <jlay...@redhat.com> | --- Hi Jeff, This version looks better. ACK. Regards, Bob Peterson Red Hat File Systems
Re: [PATCH v4 23/27] gfs2: clean up some filemap_* calls
- Original Message - | In some places, it's trying to reset the mapping error after calling | filemap_fdatawait. That's no longer required. Also, turn several | filemap_fdatawrite+filemap_fdatawait calls into filemap_write_and_wait. | That will at least return writeback errors that occur during the write | phase. | | Signed-off-by: Jeff Layton | --- Hi Jeff, This version looks better. ACK. Regards, Bob Peterson Red Hat File Systems
Re: [Cluster-devel] [PATCH 00/10] fs-DLM: Fine-tuning for several function implementations
- Original Message - | From: Markus Elfring <elfr...@users.sourceforge.net> | Date: Sat, 6 May 2017 10:15:25 +0200 | | Some update suggestions were taken into account | from static source code analysis. | | Markus Elfring (10): | Replace six seq_puts() calls by seq_putc() | Add spaces for better code readability | Improve a size determination in table_seq_start() | Use kcalloc() in dlm_scan_waiters() | Improve a size determination in dlm_recover_waiters_pre() | Delete an error message for a failed memory allocation in | dlm_recover_waiters_pre() | Use kmalloc_array() in make_member_array() | Use kcalloc() in two functions | Improve a size determination in two functions | Delete an unnecessary variable initialisation in dlm_ls_start() | | fs/dlm/debug_fs.c | 25 - | fs/dlm/lock.c | 8 +++- | fs/dlm/member.c | 15 ++- | 3 files changed, 21 insertions(+), 27 deletions(-) | | -- | 2.12.2 | | Hi Markus, I'm not a DLM maintainer, but I've reviewed your patch set and ACK them. They seem to fit the spirit of: https://www.kernel.org/doc/html/v4.10/process/coding-style.html#allocating-memory I've actually got a bunch of similar cleanups myself, but I don't think there's any overlap. Regards, Bob Peterson Red Hat File Systems
Re: [Cluster-devel] [PATCH 00/10] fs-DLM: Fine-tuning for several function implementations
- Original Message - | From: Markus Elfring | Date: Sat, 6 May 2017 10:15:25 +0200 | | Some update suggestions were taken into account | from static source code analysis. | | Markus Elfring (10): | Replace six seq_puts() calls by seq_putc() | Add spaces for better code readability | Improve a size determination in table_seq_start() | Use kcalloc() in dlm_scan_waiters() | Improve a size determination in dlm_recover_waiters_pre() | Delete an error message for a failed memory allocation in | dlm_recover_waiters_pre() | Use kmalloc_array() in make_member_array() | Use kcalloc() in two functions | Improve a size determination in two functions | Delete an unnecessary variable initialisation in dlm_ls_start() | | fs/dlm/debug_fs.c | 25 - | fs/dlm/lock.c | 8 +++- | fs/dlm/member.c | 15 ++- | 3 files changed, 21 insertions(+), 27 deletions(-) | | -- | 2.12.2 | | Hi Markus, I'm not a DLM maintainer, but I've reviewed your patch set and ACK them. They seem to fit the spirit of: https://www.kernel.org/doc/html/v4.10/process/coding-style.html#allocating-memory I've actually got a bunch of similar cleanups myself, but I don't think there's any overlap. Regards, Bob Peterson Red Hat File Systems
GFS2: Pull request (merge window)
Hi Linus,
Please disregard my previous pull request and
consider pulling the following changes for the GFS2 file system.
Special note:
This time we had an unusual dependency on a patch (f9fe1c12) which
went through David Miller's net-next tree, so we have a merge commit
in our GFS2 for-next branch. After you pulled that patch, I encountered
a strange anomaly. Despite the fact that I did a git pull of master
immediately prior, my pull request somehow generated an incorrect
diffstat which showed phantom changes we did not make, for:
block/bio.c | 12 +-
block/blk-core.c| 30 ++-
block/blk-mq-tag.c | 3 +
block/blk-mq.c | 9 +-
drivers/md/dm.c | 29 +-
drivers/md/raid10.c | 3 +-
fs/fs-writeback.c | 35 ++-
I asked around, but nobody could tell me what went wrong. Strangely,
this command:
git log --oneline --right-only origin/master...FETCH_HEAD --stat
doesn't show this, but this one does:
git diff --stat --right-only origin/master...FETCH_HEAD
So I created a temporary local branch and used git merge to
generate a correct diffstat. Hopefully this pull will just work.
If there are problems, let me know how to do it better for
next time. (Next time we hopefully won't have any merge commits).
Regards,
Bob Peterson
The following changes since commit f9fe1c12d126f9887441fa5bb165046f30ddd4b5:
rhashtable: Add rhashtable_lookup_get_insert_fast (2017-03-21 17:52:45 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git
tags/gfs2-4.12.fixes
for you to fetch changes up to ed17545d01e4a40086f1622c88ce64cc0241cfc2:
GFS2: Allow glocks to be unlocked after withdraw (2017-05-05 14:19:28 -0500)
We've got ten GFS2 patches for this merge window (plus one merge commit).
1. Andreas Gruenbacher wrote a patch to replace the deprecated
call to rhashtable_walk_init with rhashtable_walk_enter.
2. Andreas also wrote a patch to eliminate redundant code in
two of our debugfs sequence files.
3. Andreas also cleaned up the rhashtable key ugliness Linus
pointed out during this cycle, following Linus's suggestions.
4. Andreas also wrote a patch to take advantage of his new
function rhashtable_lookup_get_insert_fast. This makes glock
lookup faster and more bullet-proof.
5. Andreas also wrote a patch to revert a patch in the evict
path that caused occasional deadlocks, and is no longer
needed.
6. Andrew Price wrote a patch to re-enable fallocate for the
rindex system file to enable gfs2_grow to grow properly on
secondary file system grow operations.
7. I wrote a patch to initialize an inode number field to make
certain kernel trace points more understandable.
8. I also wrote a patch that makes GFS2 file system "withdraw"
work more like it should by ignoring operations after a
withdraw that would formerly cause a BUG() and kernel panic.
9. I also reworked the entire truncate/delete algorithm,
scrapping the old recursive algorithm in favor of a new
non-recursive algorithm. This was done for performance:
This way, GFS2 no longer needs to lock multiple resource
groups while doing truncates and deletes of files that cross
multiple resource group boundaries, allowing for better
parallelism. It also solves a problem whereby deleting large
files would request a large chunk of kernel memory, which
resulted in a get_page_from_freelist warning.
10. Due to a regression found during testing, I added a new
patch to correct "GFS2: Prevent BUG from occurring when
normal Withdraws occur".
Andreas Gruenbacher (5):
gfs2: Replace rhashtable_walk_init with rhashtable_walk_enter
gfs2: Deduplicate gfs2_{glocks,glstats}_open
gfs2: Don't pack struct lm_lockname
gfs2: Switch to rhashtable_lookup_get_insert_fast
Revert "GFS2: Wait for iopen glock dequeues"
Andrew Price (1):
gfs2: Re-enable fallocate for the rindex
Bob Peterson (5):
GFS2: Prevent BUG from occurring when normal Withdraws occur
GFS2: Temporarily zero i_no_addr when creating a dinode
Merge commit 'f9fe1c1' into for-next
GFS2: Non-recursive delete
GFS2: Allow glocks to be unlocked after withdraw
fs/gfs2/bmap.c | 741
+++
fs/gfs2/file.c | 6 +-
fs/gfs2/glock.c | 81 -
fs/gfs2/incore.h | 8 +-
fs/gfs2/inode.c | 4 +-
fs/gfs2/rgrp.c | 7 --
fs/gfs2/rgrp.h | 7 ++
fs/gfs2/super.c | 11 ++-
8 files changed, 518 insertions(+), 347 deletions(-)
GFS2: Pull request (merge window)
Hi Linus,
Please disregard my previous pull request and
consider pulling the following changes for the GFS2 file system.
Special note:
This time we had an unusual dependency on a patch (f9fe1c12) which
went through David Miller's net-next tree, so we have a merge commit
in our GFS2 for-next branch. After you pulled that patch, I encountered
a strange anomaly. Despite the fact that I did a git pull of master
immediately prior, my pull request somehow generated an incorrect
diffstat which showed phantom changes we did not make, for:
block/bio.c | 12 +-
block/blk-core.c| 30 ++-
block/blk-mq-tag.c | 3 +
block/blk-mq.c | 9 +-
drivers/md/dm.c | 29 +-
drivers/md/raid10.c | 3 +-
fs/fs-writeback.c | 35 ++-
I asked around, but nobody could tell me what went wrong. Strangely,
this command:
git log --oneline --right-only origin/master...FETCH_HEAD --stat
doesn't show this, but this one does:
git diff --stat --right-only origin/master...FETCH_HEAD
So I created a temporary local branch and used git merge to
generate a correct diffstat. Hopefully this pull will just work.
If there are problems, let me know how to do it better for
next time. (Next time we hopefully won't have any merge commits).
Regards,
Bob Peterson
The following changes since commit f9fe1c12d126f9887441fa5bb165046f30ddd4b5:
rhashtable: Add rhashtable_lookup_get_insert_fast (2017-03-21 17:52:45 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git
tags/gfs2-4.12.fixes
for you to fetch changes up to ed17545d01e4a40086f1622c88ce64cc0241cfc2:
GFS2: Allow glocks to be unlocked after withdraw (2017-05-05 14:19:28 -0500)
We've got ten GFS2 patches for this merge window (plus one merge commit).
1. Andreas Gruenbacher wrote a patch to replace the deprecated
call to rhashtable_walk_init with rhashtable_walk_enter.
2. Andreas also wrote a patch to eliminate redundant code in
two of our debugfs sequence files.
3. Andreas also cleaned up the rhashtable key ugliness Linus
pointed out during this cycle, following Linus's suggestions.
4. Andreas also wrote a patch to take advantage of his new
function rhashtable_lookup_get_insert_fast. This makes glock
lookup faster and more bullet-proof.
5. Andreas also wrote a patch to revert a patch in the evict
path that caused occasional deadlocks, and is no longer
needed.
6. Andrew Price wrote a patch to re-enable fallocate for the
rindex system file to enable gfs2_grow to grow properly on
secondary file system grow operations.
7. I wrote a patch to initialize an inode number field to make
certain kernel trace points more understandable.
8. I also wrote a patch that makes GFS2 file system "withdraw"
work more like it should by ignoring operations after a
withdraw that would formerly cause a BUG() and kernel panic.
9. I also reworked the entire truncate/delete algorithm,
scrapping the old recursive algorithm in favor of a new
non-recursive algorithm. This was done for performance:
This way, GFS2 no longer needs to lock multiple resource
groups while doing truncates and deletes of files that cross
multiple resource group boundaries, allowing for better
parallelism. It also solves a problem whereby deleting large
files would request a large chunk of kernel memory, which
resulted in a get_page_from_freelist warning.
10. Due to a regression found during testing, I added a new
patch to correct "GFS2: Prevent BUG from occurring when
normal Withdraws occur".
Andreas Gruenbacher (5):
gfs2: Replace rhashtable_walk_init with rhashtable_walk_enter
gfs2: Deduplicate gfs2_{glocks,glstats}_open
gfs2: Don't pack struct lm_lockname
gfs2: Switch to rhashtable_lookup_get_insert_fast
Revert "GFS2: Wait for iopen glock dequeues"
Andrew Price (1):
gfs2: Re-enable fallocate for the rindex
Bob Peterson (5):
GFS2: Prevent BUG from occurring when normal Withdraws occur
GFS2: Temporarily zero i_no_addr when creating a dinode
Merge commit 'f9fe1c1' into for-next
GFS2: Non-recursive delete
GFS2: Allow glocks to be unlocked after withdraw
fs/gfs2/bmap.c | 741
+++
fs/gfs2/file.c | 6 +-
fs/gfs2/glock.c | 81 -
fs/gfs2/incore.h | 8 +-
fs/gfs2/inode.c | 4 +-
fs/gfs2/rgrp.c | 7 --
fs/gfs2/rgrp.h | 7 ++
fs/gfs2/super.c | 11 ++-
8 files changed, 518 insertions(+), 347 deletions(-)
Regression in my latest GFS2 pull request
Hi Linus, I see that you haven't processed my latest GFS2 pull request. Today I found a regression in one of those patches. I've written, tested, and posted a patch to cluster-devel to fix the problem, and I'd like to get it into this merge window because it's severe. (You can't unmount a gfs2 file system after withdraw due to errors). My question is: How would you like me to handle it? Push the new patch, tag it, and do a second pull request? Wait until you've processed the first pull request, then do so? Or is there a better way? Regards, Bob Peterson
Regression in my latest GFS2 pull request
Hi Linus, I see that you haven't processed my latest GFS2 pull request. Today I found a regression in one of those patches. I've written, tested, and posted a patch to cluster-devel to fix the problem, and I'd like to get it into this merge window because it's severe. (You can't unmount a gfs2 file system after withdraw due to errors). My question is: How would you like me to handle it? Push the new patch, tag it, and do a second pull request? Wait until you've processed the first pull request, then do so? Or is there a better way? Regards, Bob Peterson
Re: [PATCH 1/2 linux-next] gfs2: check rename2 flags at once
- Original Message - | There's no need to update flags in gfs2_rename2() | This will ease whiteout addition. | | Signed-off-by: Fabian Frederick <f...@skynet.be> | --- Hi Fabian, Your patches look good. I'll push them to the for-next branch of our repo after the merge window is closed. Regards, Bob Peterson Red Hat File Systems
Re: [PATCH 1/2 linux-next] gfs2: check rename2 flags at once
- Original Message - | There's no need to update flags in gfs2_rename2() | This will ease whiteout addition. | | Signed-off-by: Fabian Frederick | --- Hi Fabian, Your patches look good. I'll push them to the for-next branch of our repo after the merge window is closed. Regards, Bob Peterson Red Hat File Systems
GFS2: Pull request (merge window)
Hi Linus,
Please consider pulling the following changes for the GFS2 file system.
Special note:
This time we had an unusual dependency on a patch (f9fe1c12) which
went through David Miller's net-next tree, so we had a merge commit
on our GFS2 for-next branch. After you pulled that patch, I encountered
a strange git anomaly. Despite the fact that I did a git pull of master
immediately prior, my pull request somehow generated an incorrect
diffstat which showed phantom changes we did not make, for:
block/bio.c | 12 +-
block/blk-core.c| 30 ++-
block/blk-mq-tag.c | 3 +
block/blk-mq.c | 9 +-
drivers/md/dm.c | 29 +-
drivers/md/raid10.c | 3 +-
fs/fs-writeback.c | 35 ++-
I asked around, but nobody could tell me what went wrong. Strangely,
this command:
git log --oneline --right-only origin/master...FETCH_HEAD --stat
didn't show it, but this one did:
git diff --stat --right-only origin/master...FETCH_HEAD
So I created a temporary local branch and used git merge to
generate a correct diffstat. Hopefully this pull will just work.
If there are problems, let me know if I should do this a different
way next time. (Next time we hopefully won't have any merge commits).
Regards,
Bob Peterson
The following changes since commit f9fe1c12d126f9887441fa5bb165046f30ddd4b5:
rhashtable: Add rhashtable_lookup_get_insert_fast (2017-03-21 17:52:45 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git
tags/gfs2-4.12.fixes
for you to fetch changes up to d552a2b9b33eafdb5eb16c643e745deb564dda7f:
GFS2: Non-recursive delete (2017-04-19 08:25:43 -0400)
We've got nine GFS2 patches for this merge window.
1. Andreas Gruenbacher wrote a patch to replace the deprecated
call to rhashtable_walk_init with rhashtable_walk_enter.
2. Andreas also wrote a patch to eliminate redundant code in
two of our debugfs sequence files.
3. Andreas also cleaned up the rhashtable key ugliness Linus
pointed out during this cycle, following Linus's suggestions.
4. Andreas also wrote a patch to take advantage of his new
function rhashtable_lookup_get_insert_fast. This makes glock
lookup faster and more bullet-proof.
5. Andreas also wrote a patch to revert a patch in the evict
path that caused occasional deadlocks, and is no longer
needed.
6. Andrew Price wrote a patch to re-enable fallocate for the
rindex system file to enable gfs2_grow to grow properly on
secondary file system grow operations.
7. I wrote a patch to initialize an inode number field to make
certain kernel trace points more understandable.
8. I also wrote a patch that makes GFS2 file system "withdraw"
work more like it should by ignoring operations after a
withdraw that would formerly cause a BUG() and kernel panic.
9. I also reworked the entire truncate/delete algorithm,
scrapping the old recursive algorithm in favor of a new
non-recursive algorithm. This was done for performance:
This way, GFS2 no longer needs to lock multiple resource
groups while doing truncates and deletes of files that cross
multiple resource group boundaries, allowing for better
parallelism. It also solves a problem whereby deleting large
files would request a large chunk of kernel memory, which
resulted in a get_page_from_freelist warning.
Andreas Gruenbacher (5):
gfs2: Replace rhashtable_walk_init with rhashtable_walk_enter
gfs2: Deduplicate gfs2_{glocks,glstats}_open
gfs2: Don't pack struct lm_lockname
gfs2: Switch to rhashtable_lookup_get_insert_fast
Revert "GFS2: Wait for iopen glock dequeues"
Andrew Price (1):
gfs2: Re-enable fallocate for the rindex
Bob Peterson (4):
GFS2: Prevent BUG from occurring when normal Withdraws occur
GFS2: Temporarily zero i_no_addr when creating a dinode
Merge commit 'f9fe1c1' into for-next
GFS2: Non-recursive delete
fs/gfs2/bmap.c | 741
fs/gfs2/file.c | 6 +-
fs/gfs2/glock.c | 77 +++---
fs/gfs2/incore.h| 8 +-
fs/gfs2/inode.c | 4 +-
fs/gfs2/rgrp.c | 7 -
fs/gfs2/rgrp.h | 7 +
fs/gfs2/super.c | 11 +-
8 files changed, 515 insertions(+), 346 deletions(-)
GFS2: Pull request (merge window)
Hi Linus,
Please consider pulling the following changes for the GFS2 file system.
Special note:
This time we had an unusual dependency on a patch (f9fe1c12) which
went through David Miller's net-next tree, so we had a merge commit
on our GFS2 for-next branch. After you pulled that patch, I encountered
a strange git anomaly. Despite the fact that I did a git pull of master
immediately prior, my pull request somehow generated an incorrect
diffstat which showed phantom changes we did not make, for:
block/bio.c | 12 +-
block/blk-core.c| 30 ++-
block/blk-mq-tag.c | 3 +
block/blk-mq.c | 9 +-
drivers/md/dm.c | 29 +-
drivers/md/raid10.c | 3 +-
fs/fs-writeback.c | 35 ++-
I asked around, but nobody could tell me what went wrong. Strangely,
this command:
git log --oneline --right-only origin/master...FETCH_HEAD --stat
didn't show it, but this one did:
git diff --stat --right-only origin/master...FETCH_HEAD
So I created a temporary local branch and used git merge to
generate a correct diffstat. Hopefully this pull will just work.
If there are problems, let me know if I should do this a different
way next time. (Next time we hopefully won't have any merge commits).
Regards,
Bob Peterson
The following changes since commit f9fe1c12d126f9887441fa5bb165046f30ddd4b5:
rhashtable: Add rhashtable_lookup_get_insert_fast (2017-03-21 17:52:45 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git
tags/gfs2-4.12.fixes
for you to fetch changes up to d552a2b9b33eafdb5eb16c643e745deb564dda7f:
GFS2: Non-recursive delete (2017-04-19 08:25:43 -0400)
We've got nine GFS2 patches for this merge window.
1. Andreas Gruenbacher wrote a patch to replace the deprecated
call to rhashtable_walk_init with rhashtable_walk_enter.
2. Andreas also wrote a patch to eliminate redundant code in
two of our debugfs sequence files.
3. Andreas also cleaned up the rhashtable key ugliness Linus
pointed out during this cycle, following Linus's suggestions.
4. Andreas also wrote a patch to take advantage of his new
function rhashtable_lookup_get_insert_fast. This makes glock
lookup faster and more bullet-proof.
5. Andreas also wrote a patch to revert a patch in the evict
path that caused occasional deadlocks, and is no longer
needed.
6. Andrew Price wrote a patch to re-enable fallocate for the
rindex system file to enable gfs2_grow to grow properly on
secondary file system grow operations.
7. I wrote a patch to initialize an inode number field to make
certain kernel trace points more understandable.
8. I also wrote a patch that makes GFS2 file system "withdraw"
work more like it should by ignoring operations after a
withdraw that would formerly cause a BUG() and kernel panic.
9. I also reworked the entire truncate/delete algorithm,
scrapping the old recursive algorithm in favor of a new
non-recursive algorithm. This was done for performance:
This way, GFS2 no longer needs to lock multiple resource
groups while doing truncates and deletes of files that cross
multiple resource group boundaries, allowing for better
parallelism. It also solves a problem whereby deleting large
files would request a large chunk of kernel memory, which
resulted in a get_page_from_freelist warning.
Andreas Gruenbacher (5):
gfs2: Replace rhashtable_walk_init with rhashtable_walk_enter
gfs2: Deduplicate gfs2_{glocks,glstats}_open
gfs2: Don't pack struct lm_lockname
gfs2: Switch to rhashtable_lookup_get_insert_fast
Revert "GFS2: Wait for iopen glock dequeues"
Andrew Price (1):
gfs2: Re-enable fallocate for the rindex
Bob Peterson (4):
GFS2: Prevent BUG from occurring when normal Withdraws occur
GFS2: Temporarily zero i_no_addr when creating a dinode
Merge commit 'f9fe1c1' into for-next
GFS2: Non-recursive delete
fs/gfs2/bmap.c | 741
fs/gfs2/file.c | 6 +-
fs/gfs2/glock.c | 77 +++---
fs/gfs2/incore.h| 8 +-
fs/gfs2/inode.c | 4 +-
fs/gfs2/rgrp.c | 7 -
fs/gfs2/rgrp.h | 7 +
fs/gfs2/super.c | 11 +-
8 files changed, 515 insertions(+), 346 deletions(-)
Re: [PATCH v3 20/20] gfs2: clean up some filemap_* calls
Hi,
- Original Message -
| On Mon, 2017-04-24 at 10:12 -0400, Bob Peterson wrote:
| > > + filemap_write_and_wait_range(mapping, gl->gl_vm.start, gl->gl_vm.end);
| >
| > This should probably have "error = ", no?
| >
|
| This error is discarded in the current code after resetting the error in
| the mapping. With the earlier patches in this set we don't need to reset
| the error like this anymore.
|
| Now, if this code should doing something else with those errors, then
| that's a separate problem.
Okay, I see. My bad.
| > > gfs2_ail_empty_gl(gl);
| > >
| > > spin_lock(>gl_lockref.lock);
| > > @@ -225,12 +223,10 @@ static void inode_go_sync(struct gfs2_glock *gl)
| > > filemap_fdatawrite(metamapping);
| > > if (ip) {
| > > struct address_space *mapping = ip->i_inode.i_mapping;
| > > - filemap_fdatawrite(mapping);
| > > - error = filemap_fdatawait(mapping);
| > > - mapping_set_error(mapping, error);
| > > + filemap_write_and_wait(mapping);
| > > + } else {
| > > + filemap_fdatawait(metamapping);
| > > }
| > > - error = filemap_fdatawait(metamapping);
| > > - mapping_set_error(metamapping, error);
| >
| > This part doesn't look right at all. There's a big difference in gfs2
| > between
| > mapping and metamapping. We need to wait for metamapping regardless.
| >
|
| ...and this should wait. Basically, filemap_write_and_wait does
| filemap_fdatawrite and then filemap_fdatawait. This is mostly just
| replacing the existing code with a more concise helper.
But this isn't a simple replacement with a helper. This is two different
address spaces (mapping and metamapping) and you added an else in there.
So with this patch metamapping gets written, and if there's an ip,
mapping gets written but it doesn't wait for metamapping. Unless
I'm missing something.
You could replace both filemap_fdatawrites with the helper instead.
Today's code is structured as:
(a) write metamapping
if (ip)
(b) write mapping
(c) wait for mapping
(d) wait for metamapping
If you use the helper for both, it becomes, (a & d)(b & c) which is probably
acceptable. (I think we just tried to optimize what the elevator was doing).
But the way you've got it coded here still looks wrong. It looks like:
(a)
if (ip)
(b & c)
ELSE -
(d)
So (d) (metamapping) isn't guaranteed to be synced at the end of the function.
Of course, you know the modified helper functions better than I do.
What am I missing?
Regards,
Bob Peterson
Red Hat File Systems
Re: [PATCH v3 20/20] gfs2: clean up some filemap_* calls
Hi,
- Original Message -
| On Mon, 2017-04-24 at 10:12 -0400, Bob Peterson wrote:
| > > + filemap_write_and_wait_range(mapping, gl->gl_vm.start, gl->gl_vm.end);
| >
| > This should probably have "error = ", no?
| >
|
| This error is discarded in the current code after resetting the error in
| the mapping. With the earlier patches in this set we don't need to reset
| the error like this anymore.
|
| Now, if this code should doing something else with those errors, then
| that's a separate problem.
Okay, I see. My bad.
| > > gfs2_ail_empty_gl(gl);
| > >
| > > spin_lock(>gl_lockref.lock);
| > > @@ -225,12 +223,10 @@ static void inode_go_sync(struct gfs2_glock *gl)
| > > filemap_fdatawrite(metamapping);
| > > if (ip) {
| > > struct address_space *mapping = ip->i_inode.i_mapping;
| > > - filemap_fdatawrite(mapping);
| > > - error = filemap_fdatawait(mapping);
| > > - mapping_set_error(mapping, error);
| > > + filemap_write_and_wait(mapping);
| > > + } else {
| > > + filemap_fdatawait(metamapping);
| > > }
| > > - error = filemap_fdatawait(metamapping);
| > > - mapping_set_error(metamapping, error);
| >
| > This part doesn't look right at all. There's a big difference in gfs2
| > between
| > mapping and metamapping. We need to wait for metamapping regardless.
| >
|
| ...and this should wait. Basically, filemap_write_and_wait does
| filemap_fdatawrite and then filemap_fdatawait. This is mostly just
| replacing the existing code with a more concise helper.
But this isn't a simple replacement with a helper. This is two different
address spaces (mapping and metamapping) and you added an else in there.
So with this patch metamapping gets written, and if there's an ip,
mapping gets written but it doesn't wait for metamapping. Unless
I'm missing something.
You could replace both filemap_fdatawrites with the helper instead.
Today's code is structured as:
(a) write metamapping
if (ip)
(b) write mapping
(c) wait for mapping
(d) wait for metamapping
If you use the helper for both, it becomes, (a & d)(b & c) which is probably
acceptable. (I think we just tried to optimize what the elevator was doing).
But the way you've got it coded here still looks wrong. It looks like:
(a)
if (ip)
(b & c)
ELSE -
(d)
So (d) (metamapping) isn't guaranteed to be synced at the end of the function.
Of course, you know the modified helper functions better than I do.
What am I missing?
Regards,
Bob Peterson
Red Hat File Systems
Re: [PATCH v3 20/20] gfs2: clean up some filemap_* calls
- Original Message -
| In some places, it's trying to reset the mapping error after calling
| filemap_fdatawait. That's no longer required. Also, turn several
| filemap_fdatawrite+filemap_fdatawait calls into filemap_write_and_wait.
| That will at least return writeback errors that occur during the write
| phase.
|
| Signed-off-by: Jeff Layton <jlay...@redhat.com>
| ---
| fs/gfs2/glops.c | 12
| fs/gfs2/lops.c | 4 +---
| fs/gfs2/super.c | 6 ++
| 3 files changed, 7 insertions(+), 15 deletions(-)
|
| diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
| index 5db59d444838..7362d19fdc4c 100644
| --- a/fs/gfs2/glops.c
| +++ b/fs/gfs2/glops.c
| @@ -158,9 +158,7 @@ static void rgrp_go_sync(struct gfs2_glock *gl)
| GLOCK_BUG_ON(gl, gl->gl_state != LM_ST_EXCLUSIVE);
|
| gfs2_log_flush(sdp, gl, NORMAL_FLUSH);
| - filemap_fdatawrite_range(mapping, gl->gl_vm.start, gl->gl_vm.end);
| - error = filemap_fdatawait_range(mapping, gl->gl_vm.start,
gl->gl_vm.end);
| - mapping_set_error(mapping, error);
| + filemap_write_and_wait_range(mapping, gl->gl_vm.start, gl->gl_vm.end);
This should probably have "error = ", no?
| gfs2_ail_empty_gl(gl);
|
| spin_lock(>gl_lockref.lock);
| @@ -225,12 +223,10 @@ static void inode_go_sync(struct gfs2_glock *gl)
| filemap_fdatawrite(metamapping);
| if (ip) {
| struct address_space *mapping = ip->i_inode.i_mapping;
| - filemap_fdatawrite(mapping);
| - error = filemap_fdatawait(mapping);
| - mapping_set_error(mapping, error);
| + filemap_write_and_wait(mapping);
| + } else {
| + filemap_fdatawait(metamapping);
| }
| - error = filemap_fdatawait(metamapping);
| - mapping_set_error(metamapping, error);
This part doesn't look right at all. There's a big difference in gfs2 between
mapping and metamapping. We need to wait for metamapping regardless.
(snip)
Regards,
Bob Peterson
Red Hat File Systems
Re: [PATCH v3 20/20] gfs2: clean up some filemap_* calls
- Original Message -
| In some places, it's trying to reset the mapping error after calling
| filemap_fdatawait. That's no longer required. Also, turn several
| filemap_fdatawrite+filemap_fdatawait calls into filemap_write_and_wait.
| That will at least return writeback errors that occur during the write
| phase.
|
| Signed-off-by: Jeff Layton
| ---
| fs/gfs2/glops.c | 12
| fs/gfs2/lops.c | 4 +---
| fs/gfs2/super.c | 6 ++
| 3 files changed, 7 insertions(+), 15 deletions(-)
|
| diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
| index 5db59d444838..7362d19fdc4c 100644
| --- a/fs/gfs2/glops.c
| +++ b/fs/gfs2/glops.c
| @@ -158,9 +158,7 @@ static void rgrp_go_sync(struct gfs2_glock *gl)
| GLOCK_BUG_ON(gl, gl->gl_state != LM_ST_EXCLUSIVE);
|
| gfs2_log_flush(sdp, gl, NORMAL_FLUSH);
| - filemap_fdatawrite_range(mapping, gl->gl_vm.start, gl->gl_vm.end);
| - error = filemap_fdatawait_range(mapping, gl->gl_vm.start,
gl->gl_vm.end);
| - mapping_set_error(mapping, error);
| + filemap_write_and_wait_range(mapping, gl->gl_vm.start, gl->gl_vm.end);
This should probably have "error = ", no?
| gfs2_ail_empty_gl(gl);
|
| spin_lock(>gl_lockref.lock);
| @@ -225,12 +223,10 @@ static void inode_go_sync(struct gfs2_glock *gl)
| filemap_fdatawrite(metamapping);
| if (ip) {
| struct address_space *mapping = ip->i_inode.i_mapping;
| - filemap_fdatawrite(mapping);
| - error = filemap_fdatawait(mapping);
| - mapping_set_error(mapping, error);
| + filemap_write_and_wait(mapping);
| + } else {
| + filemap_fdatawait(metamapping);
| }
| - error = filemap_fdatawait(metamapping);
| - mapping_set_error(metamapping, error);
This part doesn't look right at all. There's a big difference in gfs2 between
mapping and metamapping. We need to wait for metamapping regardless.
(snip)
Regards,
Bob Peterson
Red Hat File Systems
Re: Apparent backward time travel in timestamps on file creation
- Original Message - | On Thu, Mar 30, 2017 at 1:13 PM, David Howells <dhowe...@redhat.com> wrote: | > Linus Torvalds <torva...@linux-foundation.org> wrote: | > | >> The error bar can be huge, for the simple reason that the filesystem | >> you are testing may not be sharing a clock with the CPU at _all_. | >> | >> IOW, think network filesystems. | > | > Can't I just not do the tests when the filesystem is a network fs? I don't | > think it should be a problem for disk filesystems on network-attached | > storage. | | So I actually think that the whole "check timestamps" would be | interesting as a test across a lot of filesystems - including very | much network filesystems - but I think such a test should be largely | informational rather than about correctness. | | Now, there definitely are correctness issues too wrt file timestamps | (ie the whole "writes should update mtime" kind of testing), and I | think many of those cound be extended to check relative timestamps on | the same filesystem. For example, if you write to file A first, and to | file B second, it would certainly be odd and interesting if file B now | has a modification time that is before file A. This can happen, and it's not just network file systems. This issue is also a concern of GFS2 where we have shared storage. We like to think ntp will keep things relatively sane, but still, we've had issues in the past where time discrepancies have caused confusion: File X is created on node 1, but due to clock drift, node 2 sees that file as having been created in the future, etc. It's even more worrisome outside the kernel where software (e.g. in the past, parts of the cluster infrastructure) would calculate negative time values, interpret them as an "nearly infinite amount of time" having passed, and then various watchdogs nuking nodes. I remember a long time ago someone was up in arms because of weird effects they were seeing, and it boiled down to not using any time sync and one of their cluster nodes had the wrong month, or some such. Regards, Bob Peterson Red Hat File Systems
Re: Apparent backward time travel in timestamps on file creation
- Original Message - | On Thu, Mar 30, 2017 at 1:13 PM, David Howells wrote: | > Linus Torvalds wrote: | > | >> The error bar can be huge, for the simple reason that the filesystem | >> you are testing may not be sharing a clock with the CPU at _all_. | >> | >> IOW, think network filesystems. | > | > Can't I just not do the tests when the filesystem is a network fs? I don't | > think it should be a problem for disk filesystems on network-attached | > storage. | | So I actually think that the whole "check timestamps" would be | interesting as a test across a lot of filesystems - including very | much network filesystems - but I think such a test should be largely | informational rather than about correctness. | | Now, there definitely are correctness issues too wrt file timestamps | (ie the whole "writes should update mtime" kind of testing), and I | think many of those cound be extended to check relative timestamps on | the same filesystem. For example, if you write to file A first, and to | file B second, it would certainly be odd and interesting if file B now | has a modification time that is before file A. This can happen, and it's not just network file systems. This issue is also a concern of GFS2 where we have shared storage. We like to think ntp will keep things relatively sane, but still, we've had issues in the past where time discrepancies have caused confusion: File X is created on node 1, but due to clock drift, node 2 sees that file as having been created in the future, etc. It's even more worrisome outside the kernel where software (e.g. in the past, parts of the cluster infrastructure) would calculate negative time values, interpret them as an "nearly infinite amount of time" having passed, and then various watchdogs nuking nodes. I remember a long time ago someone was up in arms because of weird effects they were seeing, and it boiled down to not using any time sync and one of their cluster nodes had the wrong month, or some such. Regards, Bob Peterson Red Hat File Systems
Re: GFS2: pull request for high-priority bug
- Original Message - | On Wed, Mar 15, 2017 at 7:32 AM, Bob Peterson <rpete...@redhat.com> wrote: | > | > Andreas Gruenbacher (1): | > gfs2: Avoid alignment hole in struct lm_lockname | | So I've pulled this because I think it fixes a real bug, but honestly | I think it's the wrong fix. | | Marking that lm_lockname structure "packed, aligned(4)" means that the | compiler will now think that the 64-bit fields in it may be unaligned | - including on architectures where that can be very expensive and the | compiler now might generate stupid unaligned instruction sequences to | load those values. | | So the *correct* fix, I think, would have been: | | - add a comment about not having holes in the struct due to the hashing | | - sort the fields by size (so "ln_number" first, then "ln_sbd", then | "ln_type") | | - use offsetofend(struct lm_lockname, ln_type) instead of sizeof() when | hashing | | which avoids the "possibly generate garbage code" issue due to the | quick-and-dirty one-liner approach. | | Hmm? | | Linus Hi Linus, Thanks. Yes, good ideas. I see your point and I'll see if we can get that fixed up for the next merge window. Bob Peterson
Re: GFS2: pull request for high-priority bug
- Original Message - | On Wed, Mar 15, 2017 at 7:32 AM, Bob Peterson wrote: | > | > Andreas Gruenbacher (1): | > gfs2: Avoid alignment hole in struct lm_lockname | | So I've pulled this because I think it fixes a real bug, but honestly | I think it's the wrong fix. | | Marking that lm_lockname structure "packed, aligned(4)" means that the | compiler will now think that the 64-bit fields in it may be unaligned | - including on architectures where that can be very expensive and the | compiler now might generate stupid unaligned instruction sequences to | load those values. | | So the *correct* fix, I think, would have been: | | - add a comment about not having holes in the struct due to the hashing | | - sort the fields by size (so "ln_number" first, then "ln_sbd", then | "ln_type") | | - use offsetofend(struct lm_lockname, ln_type) instead of sizeof() when | hashing | | which avoids the "possibly generate garbage code" issue due to the | quick-and-dirty one-liner approach. | | Hmm? | | Linus Hi Linus, Thanks. Yes, good ideas. I see your point and I'll see if we can get that fixed up for the next merge window. Bob Peterson
GFS2: pull request for high-priority bug
Hi Linus, Please consider pulling the following additional patch for the GFS2 file system. Regards, Bob Peterson The following changes since commit ae50dfd61665086e617cc9e554a1285d52765670: Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2017-03-14 21:31:23 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.11-rc3.fixes for you to fetch changes up to 28ea06c46fbcab63fd9a55531387b7928a18a590: gfs2: Avoid alignment hole in struct lm_lockname (2017-03-15 10:06:07 -0400) This is an emergency patch for 4.11-rc3 The GFS2 developers uncovered a really nasty problem that can lead to random corruption and kernel panic, much like the last one. Andreas Gruenbacher wrote this simple one-line patch to fix the problem: 28ea06c gfs2: Avoid alignment hole in struct lm_lockname Andreas Gruenbacher (1): gfs2: Avoid alignment hole in struct lm_lockname fs/gfs2/incore.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
GFS2: pull request for high-priority bug
Hi Linus, Please consider pulling the following additional patch for the GFS2 file system. Regards, Bob Peterson The following changes since commit ae50dfd61665086e617cc9e554a1285d52765670: Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2017-03-14 21:31:23 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.11-rc3.fixes for you to fetch changes up to 28ea06c46fbcab63fd9a55531387b7928a18a590: gfs2: Avoid alignment hole in struct lm_lockname (2017-03-15 10:06:07 -0400) This is an emergency patch for 4.11-rc3 The GFS2 developers uncovered a really nasty problem that can lead to random corruption and kernel panic, much like the last one. Andreas Gruenbacher wrote this simple one-line patch to fix the problem: 28ea06c gfs2: Avoid alignment hole in struct lm_lockname Andreas Gruenbacher (1): gfs2: Avoid alignment hole in struct lm_lockname fs/gfs2/incore.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
GFS2: Pull request (merge window) addendum
Hi Linus, Please consider pulling the following additional patch for the GFS2 file system. Sorry for the lack of bake time on this one, but it is high priority, simple, and has been well tested. Bob Peterson The following changes since commit c548a1c175608e268f6495f3f82461303584d1c9: gfs2: Make gfs2_write_full_page static (2017-02-03 08:23:47 -0500) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.11.addendum for you to fetch changes up to f38e5fb95a1f8feda88531eedc98f69b24748712: gfs2: Add missing rcu locking for glock lookup (2017-02-23 10:06:00 -0500) This is an addendum for the 4.11 merge window. Andy Price wrote this patch to close a nasty race condition that allows access to glocks that are being destroyed. Without this patch, GFS2 is vulnerable to random corruption and kernel panic. Andrew Price (1): gfs2: Add missing rcu locking for glock lookup fs/gfs2/glock.c | 5 + 1 file changed, 5 insertions(+)
GFS2: Pull request (merge window) addendum
Hi Linus, Please consider pulling the following additional patch for the GFS2 file system. Sorry for the lack of bake time on this one, but it is high priority, simple, and has been well tested. Bob Peterson The following changes since commit c548a1c175608e268f6495f3f82461303584d1c9: gfs2: Make gfs2_write_full_page static (2017-02-03 08:23:47 -0500) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.11.addendum for you to fetch changes up to f38e5fb95a1f8feda88531eedc98f69b24748712: gfs2: Add missing rcu locking for glock lookup (2017-02-23 10:06:00 -0500) This is an addendum for the 4.11 merge window. Andy Price wrote this patch to close a nasty race condition that allows access to glocks that are being destroyed. Without this patch, GFS2 is vulnerable to random corruption and kernel panic. Andrew Price (1): gfs2: Add missing rcu locking for glock lookup fs/gfs2/glock.c | 5 + 1 file changed, 5 insertions(+)
GFS2: Pull request (merge window)
Hi Linus, Please consider pulling the following changes for the GFS2 file system. Bob Peterson The following changes since commit a3443cda5588985a2724d6d0f4a5f04e625be6eb: Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security (2016-10-04 14:48:27 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.11.fixes for you to fetch changes up to c548a1c175608e268f6495f3f82461303584d1c9: gfs2: Make gfs2_write_full_page static (2017-02-03 08:23:47 -0500) We've got eight GFS2 patches for this merge window: 1. Andy Price submitted a patch to make gfs2_write_full_page a static function. 2. Dan Carpenter submitted a patch to fix a ERR_PTR thinko. I've also got a few patches, three of which fix bugs related to deleting very large files, which cause GFS2 to run out of journal space: 3. The first one prevents GFS2 delete operation from requesting too much journal space. 4. The second one fixes a problem whereby GFS2 can hang because it wasn't taking journal space demand into its calculations. 5. The third one wakes up IO waiters when a flush is done to restart processes stuck waiting for journal space to become available. The other three patches are a performance improvement related to spin_lock contention between multiple writers: 6. The "tr_touched" variable was switched to a flag to be more atomic and eliminate the possibility of some races. 7. Function meta_lo_add was moved inline with its only caller to make the code more readable and efficient. 8. Contention on the gfs2_log_lock spinlock was greatly reduced by avoiding the lock altogether in cases where we don't really need it: buffers that already appear in the appropriate metadata list for the journal. Many thanks to Steve Whitehouse for the ideas and principles behind these patches. Andrew Price (1): gfs2: Make gfs2_write_full_page static Bob Peterson (6): GFS2: Limit number of transaction blocks requested for truncates GFS2: Made logd daemon take into account log demand GFS2: Wake up io waiters whenever a flush is done GFS2: Switch tr_touched to flag in transaction GFS2: Inline function meta_lo_add GFS2: Reduce contention on gfs2_log_lock Dan Carpenter (1): GFS2: Fix reference to ERR_PTR in gfs2_glock_iter_next fs/gfs2/aops.c | 4 +-- fs/gfs2/bmap.c | 29 +-- fs/gfs2/glock.c | 12 fs/gfs2/incore.h | 11 +-- fs/gfs2/log.c| 21 ++ fs/gfs2/meta_io.c| 6 ++-- fs/gfs2/ops_fstype.c | 1 + fs/gfs2/trans.c | 81 8 files changed, 105 insertions(+), 60 deletions(-)
GFS2: Pull request (merge window)
Hi Linus, Please consider pulling the following changes for the GFS2 file system. Bob Peterson The following changes since commit a3443cda5588985a2724d6d0f4a5f04e625be6eb: Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security (2016-10-04 14:48:27 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git tags/gfs2-4.11.fixes for you to fetch changes up to c548a1c175608e268f6495f3f82461303584d1c9: gfs2: Make gfs2_write_full_page static (2017-02-03 08:23:47 -0500) We've got eight GFS2 patches for this merge window: 1. Andy Price submitted a patch to make gfs2_write_full_page a static function. 2. Dan Carpenter submitted a patch to fix a ERR_PTR thinko. I've also got a few patches, three of which fix bugs related to deleting very large files, which cause GFS2 to run out of journal space: 3. The first one prevents GFS2 delete operation from requesting too much journal space. 4. The second one fixes a problem whereby GFS2 can hang because it wasn't taking journal space demand into its calculations. 5. The third one wakes up IO waiters when a flush is done to restart processes stuck waiting for journal space to become available. The other three patches are a performance improvement related to spin_lock contention between multiple writers: 6. The "tr_touched" variable was switched to a flag to be more atomic and eliminate the possibility of some races. 7. Function meta_lo_add was moved inline with its only caller to make the code more readable and efficient. 8. Contention on the gfs2_log_lock spinlock was greatly reduced by avoiding the lock altogether in cases where we don't really need it: buffers that already appear in the appropriate metadata list for the journal. Many thanks to Steve Whitehouse for the ideas and principles behind these patches. Andrew Price (1): gfs2: Make gfs2_write_full_page static Bob Peterson (6): GFS2: Limit number of transaction blocks requested for truncates GFS2: Made logd daemon take into account log demand GFS2: Wake up io waiters whenever a flush is done GFS2: Switch tr_touched to flag in transaction GFS2: Inline function meta_lo_add GFS2: Reduce contention on gfs2_log_lock Dan Carpenter (1): GFS2: Fix reference to ERR_PTR in gfs2_glock_iter_next fs/gfs2/aops.c | 4 +-- fs/gfs2/bmap.c | 29 +-- fs/gfs2/glock.c | 12 fs/gfs2/incore.h | 11 +-- fs/gfs2/log.c| 21 ++ fs/gfs2/meta_io.c| 6 ++-- fs/gfs2/ops_fstype.c | 1 + fs/gfs2/trans.c | 81 8 files changed, 105 insertions(+), 60 deletions(-)
Re: CONFIG_VMAP_STACK, on-stack struct, and wake_up_bit
- Original Message - | I mean, it would be great if you try a couple times but even if you're | unsuccessful, that's fine too - the fix is obviously correct and I've | confirmed that it boots fine in my VM here. Hi Boris, I rebooted the machine with and without your patch, about 15 times each, and no failures. Not sure why I got it the first time. Must have been a one-off. Bob Peterson
