Re: [Cluster-devel] [PATCH dlm/next 2/2] fs: dlm: handle -EINVAL as log_error()
Hi,
On Tue, Jul 19, 2022 at 9:15 PM Alexander Aring wrote:
>
> If the user generates a -EINVAL it's probably because the user using DLM
> wrong. To give the user notice about that wrong behaviour we should
> always print -EINVAL errors on the proper loglevel. In case of other
> errors like -EBUSY it will be still printed on debug loglevel as the
> current API handles it as "retry again".
>
> Signed-off-by: Alexander Aring
> ---
> fs/dlm/lock.c | 24 ++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
> index d8de4003ec6a..7d5f94867e45 100644
> --- a/fs/dlm/lock.c
> +++ b/fs/dlm/lock.c
> @@ -2900,11 +2900,21 @@ static int validate_lock_args(struct dlm_ls *ls,
> struct dlm_lkb *lkb,
> #endif
> rv = 0;
> out:
> - if (rv)
> + switch (rv) {
> + case -EINVAL:
> + log_error(ls, "%s %d %x %x %x %d %d %s", __func__,
> + rv, lkb->lkb_id, lkb->lkb_flags, args->flags,
> + lkb->lkb_status, lkb->lkb_wait_type,
> + lkb->lkb_resource->res_name);
> + break;
> + default:
> log_debug(ls, "%s %d %x %x %x %d %d %s", __func__,
> rv, lkb->lkb_id, lkb->lkb_flags, args->flags,
> lkb->lkb_status, lkb->lkb_wait_type,
> lkb->lkb_resource->res_name);
> + break;
> + }
> +
> return rv;
> }
>
> @@ -3037,11 +3047,21 @@ static int validate_unlock_args(struct dlm_lkb *lkb,
> struct dlm_args *args)
> lkb->lkb_astparam = args->astparam;
> rv = 0;
> out:
> - if (rv)
> + switch (rv) {
> + case -EINVAL:
> + log_error(ls, "%s %d %x %x %x %x %d %s", __func__, rv,
> + lkb->lkb_id, lkb->lkb_flags, lkb->lkb_exflags,
> + args->flags, lkb->lkb_wait_type,
> + lkb->lkb_resource->res_name);
> + break;
> + default:
> log_debug(ls, "%s %d %x %x %x %x %d %s", __func__, rv,
> lkb->lkb_id, lkb->lkb_flags, lkb->lkb_exflags,
> args->flags, lkb->lkb_wait_type,
> lkb->lkb_resource->res_name);
> + break;
> + }
> +
there need to be a case 0: which does nothing of course... will send a v2.
- Alex
[Cluster-devel] [PATCHv2 dlm/next 2/2] fs: dlm: handle -EINVAL as log_error()
If the user generates a -EINVAL it's probably because the user using DLM
wrong. To give the user notice about that wrong behaviour we should
always print -EINVAL errors on the proper loglevel. In case of other
errors like -EBUSY it will be still printed on debug loglevel as the
current API handles it as "retry again".
Signed-off-by: Alexander Aring
---
changes since v2:
- add case 0: for successful case in we don't need to print out
anything.
fs/dlm/lock.c | 28 ++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index 026c203ff529..061fa96fc978 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -2900,11 +2900,23 @@ static int validate_lock_args(struct dlm_ls *ls, struct
dlm_lkb *lkb,
#endif
rv = 0;
out:
- if (rv)
+ switch (rv) {
+ case 0:
+ break;
+ case -EINVAL:
+ log_error(ls, "%s %d %x %x %x %d %d %s", __func__,
+ rv, lkb->lkb_id, lkb->lkb_flags, args->flags,
+ lkb->lkb_status, lkb->lkb_wait_type,
+ lkb->lkb_resource->res_name);
+ break;
+ default:
log_debug(ls, "%s %d %x %x %x %d %d %s", __func__,
rv, lkb->lkb_id, lkb->lkb_flags, args->flags,
lkb->lkb_status, lkb->lkb_wait_type,
lkb->lkb_resource->res_name);
+ break;
+ }
+
return rv;
}
@@ -3037,11 +3049,23 @@ static int validate_unlock_args(struct dlm_lkb *lkb,
struct dlm_args *args)
lkb->lkb_astparam = args->astparam;
rv = 0;
out:
- if (rv)
+ switch (rv) {
+ case 0:
+ break;
+ case -EINVAL:
+ log_error(ls, "%s %d %x %x %x %x %d %s", __func__, rv,
+ lkb->lkb_id, lkb->lkb_flags, lkb->lkb_exflags,
+ args->flags, lkb->lkb_wait_type,
+ lkb->lkb_resource->res_name);
+ break;
+ default:
log_debug(ls, "%s %d %x %x %x %x %d %s", __func__, rv,
lkb->lkb_id, lkb->lkb_flags, lkb->lkb_exflags,
args->flags, lkb->lkb_wait_type,
lkb->lkb_resource->res_name);
+ break;
+ }
+
return rv;
}
--
2.31.1
[Cluster-devel] [PATCHv2 dlm/next 1/2] fs: dlm: use __func__ for function name
There are several times of using hard-coded function names inside the format string. When changing code checkpatch will drop a warning about this. This patch prepares to not dropping a checkpatch warning when introduce the same log message for a different loglevel by using __func__ instead of a hard-coded function name. Signed-off-by: Alexander Aring --- fs/dlm/lock.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c index 16d339d383cd..026c203ff529 100644 --- a/fs/dlm/lock.c +++ b/fs/dlm/lock.c @@ -2901,7 +2901,7 @@ static int validate_lock_args(struct dlm_ls *ls, struct dlm_lkb *lkb, rv = 0; out: if (rv) - log_debug(ls, "validate_lock_args %d %x %x %x %d %d %s", + log_debug(ls, "%s %d %x %x %x %d %d %s", __func__, rv, lkb->lkb_id, lkb->lkb_flags, args->flags, lkb->lkb_status, lkb->lkb_wait_type, lkb->lkb_resource->res_name); @@ -3038,7 +3038,7 @@ static int validate_unlock_args(struct dlm_lkb *lkb, struct dlm_args *args) rv = 0; out: if (rv) - log_debug(ls, "validate_unlock_args %d %x %x %x %x %d %s", rv, + log_debug(ls, "%s %d %x %x %x %x %d %s", __func__, rv, lkb->lkb_id, lkb->lkb_flags, lkb->lkb_exflags, args->flags, lkb->lkb_wait_type, lkb->lkb_resource->res_name); -- 2.31.1
[Cluster-devel] [PATCH dlm/next] fs: dlm: handle -EBUSY as first for unlock
This patch checks on -EBUSY for dlm_unlock() for non CANCEL or
FORCEUNLOCK case validation at first. Similar like it's done for
dlm_lock(). Although the current way looks okay we should anyway
moving the -EBUSY check at first after doing a check on -EINVAL
regarding to the lkb state. If new -EINVAL checks are added it
should be considered that some lkb fields are in a stable state
only when the lkb is in a non -EBUSY state. This patch is trying to
avoid such future mistake.
Signed-off-by: Alexander Aring
---
fs/dlm/lock.c | 44 ++--
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index 7d5f94867e45..75313435b39d 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -2928,23 +2928,12 @@ static int validate_lock_args(struct dlm_ls *ls, struct
dlm_lkb *lkb,
static int validate_unlock_args(struct dlm_lkb *lkb, struct dlm_args *args)
{
struct dlm_ls *ls = lkb->lkb_resource->res_ls;
- int rv = -EINVAL;
-
- if (lkb->lkb_flags & DLM_IFL_MSTCPY) {
- log_error(ls, "unlock on MSTCPY %x", lkb->lkb_id);
- dlm_print_lkb(lkb);
- goto out;
- }
-
- /* an lkb may still exist even though the lock is EOL'ed due to a
- cancel, unlock or failed noqueue request; an app can't use these
- locks; return same error as if the lkid had not been found at all */
+ int rv = -EBUSY;
- if (lkb->lkb_flags & DLM_IFL_ENDOFLIFE) {
- log_debug(ls, "unlock on ENDOFLIFE %x", lkb->lkb_id);
- rv = -ENOENT;
+ /* normal unlock not allowed if there's any op in progress */
+ if (!(args->flags & (DLM_LKF_CANCEL | DLM_LKF_FORCEUNLOCK)) &&
+ (lkb->lkb_wait_type || lkb->lkb_wait_count))
goto out;
- }
/* an lkb may be waiting for an rsb lookup to complete where the
lookup was initiated by another lock */
@@ -2959,7 +2948,24 @@ static int validate_unlock_args(struct dlm_lkb *lkb,
struct dlm_args *args)
unhold_lkb(lkb); /* undoes create_lkb() */
}
/* caller changes -EBUSY to 0 for CANCEL and FORCEUNLOCK */
- rv = -EBUSY;
+ goto out;
+ }
+
+ rv = -EINVAL;
+ if (lkb->lkb_flags & DLM_IFL_MSTCPY) {
+ log_error(ls, "unlock on MSTCPY %x", lkb->lkb_id);
+ dlm_print_lkb(lkb);
+ goto out;
+ }
+
+ /* an lkb may still exist even though the lock is EOL'ed due to a
+* cancel, unlock or failed noqueue request; an app can't use these
+* locks; return same error as if the lkid had not been found at all
+*/
+
+ if (lkb->lkb_flags & DLM_IFL_ENDOFLIFE) {
+ log_debug(ls, "unlock on ENDOFLIFE %x", lkb->lkb_id);
+ rv = -ENOENT;
goto out;
}
@@ -3032,14 +3038,8 @@ static int validate_unlock_args(struct dlm_lkb *lkb,
struct dlm_args *args)
goto out;
}
/* add_to_waiters() will set OVERLAP_UNLOCK */
- goto out_ok;
}
- /* normal unlock not allowed if there's any op in progress */
- rv = -EBUSY;
- if (lkb->lkb_wait_type || lkb->lkb_wait_count)
- goto out;
-
out_ok:
/* an overlapping op shouldn't blow away exflags from other op */
lkb->lkb_exflags |= args->flags;
--
2.31.1
[Cluster-devel] [syzbot] KASAN: invalid-free in free_prealloced_shrinker
Hello,
syzbot found the following issue on:
HEAD commit:cb71b93c2dc3 Add linux-next specific files for 20220628
git tree: linux-next
console+strace: https://syzkaller.appspot.com/x/log.txt?x=1362115208
kernel config: https://syzkaller.appspot.com/x/.config?x=badbc1adb2d582eb
dashboard link: https://syzkaller.appspot.com/bug?extid=8b481578352d4637f510
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for
Debian) 2.35.2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=150c25fc08
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1308956208
The issue was bisected to:
commit bec0918551a79c3c6b63a493a80e35e8b402804f
Author: Roman Gushchin
Date: Wed Jun 1 03:22:24 2022 +
mm: shrinkers: provide shrinkers with names
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=17451fd008
final oops: https://syzkaller.appspot.com/x/report.txt?x=14c51fd008
console output: https://syzkaller.appspot.com/x/log.txt?x=10c51fd008
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+8b481578352d4637f...@syzkaller.appspotmail.com
Fixes: bec0918551a7 ("mm: shrinkers: provide shrinkers with names")
==
BUG: KASAN: double-free in slab_free mm/slub.c:3534 [inline]
BUG: KASAN: double-free in kfree+0xe2/0x4d0 mm/slub.c:4562
CPU: 0 PID: 3647 Comm: syz-executor232 Not tainted
5.19.0-rc4-next-20220628-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google
06/29/2022
Call Trace:
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
print_address_description mm/kasan/report.c:317 [inline]
print_report.cold+0x2ba/0x719 mm/kasan/report.c:433
kasan_report_invalid_free+0x8f/0x1a0 mm/kasan/report.c:462
kasan_slab_free+0x18b/0x1c0 mm/kasan/common.c:355
kasan_slab_free include/linux/kasan.h:200 [inline]
slab_free_hook mm/slub.c:1754 [inline]
slab_free_freelist_hook+0x8b/0x1c0 mm/slub.c:1780
slab_free mm/slub.c:3534 [inline]
kfree+0xe2/0x4d0 mm/slub.c:4562
kfree_const+0x51/0x60 mm/util.c:41
free_prealloced_shrinker+0x32/0x160 mm/vmscan.c:658
destroy_unused_super.part.0+0x106/0x170 fs/super.c:185
destroy_unused_super fs/super.c:278 [inline]
alloc_super+0x8bd/0xaa0 fs/super.c:277
sget_fc+0x13e/0x7c0 fs/super.c:530
vfs_get_super fs/super.c:1134 [inline]
get_tree_nodev+0x24/0x1d0 fs/super.c:1169
vfs_get_tree+0x89/0x2f0 fs/super.c:1501
do_new_mount fs/namespace.c:3040 [inline]
path_mount+0x1320/0x1fa0 fs/namespace.c:3370
do_mount fs/namespace.c:3383 [inline]
__do_sys_mount fs/namespace.c:3591 [inline]
__se_sys_mount fs/namespace.c:3568 [inline]
__x64_sys_mount+0x27f/0x300 fs/namespace.c:3568
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x46/0xb0
RIP: 0033:0x7f84280f4ef9
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 51 15 00 00 90 48 89 f8 48 89 f7 48
89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01
c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:7ffc55338338 EFLAGS: 0246 ORIG_RAX: 00a5
RAX: ffda RBX: 0003 RCX: 7f84280f4ef9
RDX: 20c0 RSI: 2080 RDI:
RBP: 7ffc55338360 R08: R09: 7ffc55338370
R10: R11: 0246 R12: 0003
R13: 7ffc55338380 R14: 7ffc553383c0 R15: 0006
Allocated by task 143:
kasan_save_stack+0x1e/0x40 mm/kasan/common.c:38
kasan_set_track mm/kasan/common.c:45 [inline]
set_alloc_info mm/kasan/common.c:436 [inline]
kasan_kmalloc mm/kasan/common.c:515 [inline]
kasan_kmalloc mm/kasan/common.c:474 [inline]
__kasan_kmalloc+0xa9/0xd0 mm/kasan/common.c:524
kmalloc include/linux/slab.h:605 [inline]
kzalloc include/linux/slab.h:733 [inline]
rh_call_control drivers/usb/core/hcd.c:514 [inline]
rh_urb_enqueue drivers/usb/core/hcd.c:848 [inline]
usb_hcd_submit_urb+0x661/0x2220 drivers/usb/core/hcd.c:1551
usb_submit_urb+0x86d/0x1880 drivers/usb/core/urb.c:594
usb_start_wait_urb+0x101/0x4b0 drivers/usb/core/message.c:58
usb_internal_control_msg drivers/usb/core/message.c:102 [inline]
usb_control_msg+0x31c/0x4a0 drivers/usb/core/message.c:153
get_port_status drivers/usb/core/hub.c:580 [inline]
hub_ext_port_status+0x112/0x450 drivers/usb/core/hub.c:597
usb_hub_port_status drivers/usb/core/hub.c:619 [inline]
hub_activate+0xa5c/0x1c90 drivers/usb/core/hub.c:1129
process_one_work+0x991/0x1610 kernel/workqueue.c:2289
worker_thread+0x665/0x1080 kernel/workqueue.c:2436
kthread+0x2e9/0x3a0 kernel/kthread.c:376
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:302
Freed by task 3647:
kasan_save_stack+0x1e/0x40 mm/kasan/common.c:38
kasan_set_track+0x21/0x30 mm/kasan/common.c:45
kasan_set_free_info+0x20/0x30 mm/kasan/generic.c:370
