* [PATCH 07/11] touchscreen: mc13783: Use octal permissions
From: dev-harsh1998 @ 2018-07-23 13:19 UTC (permalink / raw)
To: trivial
Cc: Harshit Jain, Dmitry Torokhov, Simon Budig, Andi Shyti,
Luca Ceresoli, linux-input, linux-kernel
In-Reply-To: <20180723131954.30739-1-harshitjain6751@gmail.com>
WARNING: Symbolic permissions 'S_IRUGO | S_IWUSR' are not preferred. Consider using octal permissions '0644'.
+module_param(sample_tolerance, uint, S_IRUGO | S_IWUSR);
Signed-off-by: Harshit Jain <harshitjain6751@gmail.com>
---
drivers/input/touchscreen/mc13783_ts.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/input/touchscreen/mc13783_ts.c b/drivers/input/touchscreen/mc13783_ts.c
index ef64f36c5ffc..4e33349f101e 100644
--- a/drivers/input/touchscreen/mc13783_ts.c
+++ b/drivers/input/touchscreen/mc13783_ts.c
@@ -25,9 +25,9 @@
#define DEFAULT_SAMPLE_TOLERANCE 300
static unsigned int sample_tolerance = DEFAULT_SAMPLE_TOLERANCE;
-module_param(sample_tolerance, uint, S_IRUGO | S_IWUSR);
+module_param(sample_tolerance, uint, 0644);
MODULE_PARM_DESC(sample_tolerance,
- "If the minimal and maximal value read out for one axis (out "
+ "If the minimal and maximum value read out for one axis (out "
"of three) differ by this value (default: "
__stringify(DEFAULT_SAMPLE_TOLERANCE) ") or more, the reading "
"is supposed to be wrong and is discarded. Set to 0 to "
--
2.17.1
^ permalink raw reply related
* [PATCH 06/11] touchscreen: ili210x: Use octal permissions
From: dev-harsh1998 @ 2018-07-23 13:19 UTC (permalink / raw)
To: trivial
Cc: Harshit Jain, Dmitry Torokhov, Andi Shyti, Simon Budig,
Luca Ceresoli, linux-input, linux-kernel
In-Reply-To: <20180723131954.30739-1-harshitjain6751@gmail.com>
WARNING: Symbolic permissions 'S_IWUSR' are not preferred. Consider using octal permissions '0200'.
+static DEVICE_ATTR(calibrate, S_IWUSR, NULL, ili210x_calibrate);
Signed-off-by: Harshit Jain <harshitjain6751@gmail.com>
---
drivers/input/touchscreen/ili210x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
index 6f76eeedf465..07526a6f2cb7 100644
--- a/drivers/input/touchscreen/ili210x.c
+++ b/drivers/input/touchscreen/ili210x.c
@@ -169,7 +169,7 @@ static ssize_t ili210x_calibrate(struct device *dev,
return count;
}
-static DEVICE_ATTR(calibrate, S_IWUSR, NULL, ili210x_calibrate);
+static DEVICE_ATTR(calibrate, 0200, NULL, ili210x_calibrate);
static struct attribute *ili210x_attributes[] = {
&dev_attr_calibrate.attr,
--
2.17.1
^ permalink raw reply related
* [PATCH 05/11] touchscreen: elants: Use octal permissions
From: dev-harsh1998 @ 2018-07-23 13:19 UTC (permalink / raw)
To: trivial
Cc: Harshit Jain, Dmitry Torokhov, Simon Budig, Andi Shyti,
Luca Ceresoli, Greg Kroah-Hartman, Joe Perches, Guenter Roeck,
linux-input, linux-kernel
In-Reply-To: <20180723131954.30739-1-harshitjain6751@gmail.com>
WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
+static DEVICE_ATTR(iap_mode, S_IRUGO, show_iap_mode, NULL);
WARNING: Symbolic permissions 'S_IWUSR' are not preferred. Consider using octal permissions '0200'.
+static DEVICE_ATTR(update_fw, S_IWUSR, NULL, write_update_fw)
WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
+ .dattr = __ATTR(_field, S_IRUGO, \
Signed-off-by: Harshit Jain <harshitjain6751@gmail.com>
---
drivers/input/touchscreen/elants_i2c.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
index d21ca39b0fdb..9717b7c3bb6f 100644
--- a/drivers/input/touchscreen/elants_i2c.c
+++ b/drivers/input/touchscreen/elants_i2c.c
@@ -1000,8 +1000,8 @@ static ssize_t show_iap_mode(struct device *dev,
}
static DEVICE_ATTR_WO(calibrate);
-static DEVICE_ATTR(iap_mode, S_IRUGO, show_iap_mode, NULL);
-static DEVICE_ATTR(update_fw, S_IWUSR, NULL, write_update_fw);
+static DEVICE_ATTR(iap_mode, 0444, show_iap_mode, NULL);
+static DEVICE_ATTR(update_fw, 0200, NULL, write_update_fw);
struct elants_version_attribute {
struct device_attribute dattr;
@@ -1016,7 +1016,7 @@ struct elants_version_attribute {
__ELANTS_FIELD_SIZE(_field))
#define ELANTS_VERSION_ATTR(_field) \
struct elants_version_attribute elants_ver_attr_##_field = { \
- .dattr = __ATTR(_field, S_IRUGO, \
+ .dattr = __ATTR(_field, 0444, \
elants_version_attribute_show, NULL), \
.field_offset = offsetof(struct elants_data, _field), \
.field_size = __ELANTS_VERIFY_SIZE(_field), \
--
2.17.1
^ permalink raw reply related
* [PATCH 04/11] touchscreen: amtel: Use octal permissions
From: dev-harsh1998 @ 2018-07-23 13:19 UTC (permalink / raw)
To: trivial
Cc: Harshit Jain, Nick Dyer, Dmitry Torokhov, Simon Budig, Andi Shyti,
Luca Ceresoli, linux-input, linux-kernel
In-Reply-To: <20180723131954.30739-1-harshitjain6751@gmail.com>
WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
+static DEVICE_ATTR(fw_version, S_IRUGO, mxt_fw_version_show, NULL);
WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
+static DEVICE_ATTR(hw_version, S_IRUGO, mxt_hw_version_show, NULL);
WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
+static DEVICE_ATTR(object, S_IRUGO, mxt_object_show, NULL);
WARNING: Symbolic permissions 'S_IWUSR' are not preferred. Consider using octal permissions '0200'.
+static DEVICE_ATTR(update_fw, S_IWUSR, NULL, mxt_update_fw_store);
Signed-off-by: Harshit Jain <harshitjain6751@gmail.com>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 54fe190fd4bc..5202a8244f1d 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -2892,10 +2892,10 @@ static ssize_t mxt_update_fw_store(struct device *dev,
return count;
}
-static DEVICE_ATTR(fw_version, S_IRUGO, mxt_fw_version_show, NULL);
-static DEVICE_ATTR(hw_version, S_IRUGO, mxt_hw_version_show, NULL);
-static DEVICE_ATTR(object, S_IRUGO, mxt_object_show, NULL);
-static DEVICE_ATTR(update_fw, S_IWUSR, NULL, mxt_update_fw_store);
+static DEVICE_ATTR(fw_version, 0444, mxt_fw_version_show, NULL);
+static DEVICE_ATTR(hw_version, 0444, mxt_hw_version_show, NULL);
+static DEVICE_ATTR(object, 0444, mxt_object_show, NULL);
+static DEVICE_ATTR(update_fw, 0200, NULL, mxt_update_fw_store);
static struct attribute *mxt_attrs[] = {
&dev_attr_fw_version.attr,
--
2.17.1
^ permalink raw reply related
* [PATCH 03/11] touchscreen: ad7846: Use octal permissions
From: dev-harsh1998 @ 2018-07-23 13:19 UTC (permalink / raw)
To: trivial
Cc: Harshit Jain, Dmitry Torokhov, Simon Budig, Andi Shyti,
Luca Ceresoli, linux-input, linux-kernel
In-Reply-To: <20180723131954.30739-1-harshitjain6751@gmail.com>
WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
+static DEVICE_ATTR(name, S_IRUGO, name ## _show, NULL);
WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
+static DEVICE_ATTR(pen_down, S_IRUGO, ads7846_pen_down_show, NULL);
Signed-off-by: Harshit Jain <harshitjain6751@gmail.com>
---
drivers/input/touchscreen/ads7846.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index a2f45aefce08..7c07de0802de 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -431,7 +431,7 @@ name ## _show(struct device *dev, struct device_attribute *attr, char *buf) \
return v; \
return sprintf(buf, "%u\n", adjust(ts, v)); \
} \
-static DEVICE_ATTR(name, S_IRUGO, name ## _show, NULL);
+static DEVICE_ATTR(name, 0444, name ## _show, NULL);
/* Sysfs conventions report temperatures in millidegrees Celsius.
@@ -561,7 +561,7 @@ static ssize_t ads7846_pen_down_show(struct device *dev,
return sprintf(buf, "%u\n", ts->pendown);
}
-static DEVICE_ATTR(pen_down, S_IRUGO, ads7846_pen_down_show, NULL);
+static DEVICE_ATTR(pen_down, 0444, ads7846_pen_down_show, NULL);
static ssize_t ads7846_disable_show(struct device *dev,
struct device_attribute *attr, char *buf)
--
2.17.1
^ permalink raw reply related
* [PATCH 02/11] touchscreen: ad7877: Use octal permissions
From: dev-harsh1998 @ 2018-07-23 13:19 UTC (permalink / raw)
To: trivial
Cc: Harshit Jain, Michael Hennerich, Dmitry Torokhov, Simon Budig,
Andi Shyti, Luca Ceresoli, linux-input, linux-kernel
WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
+static DEVICE_ATTR(name, S_IRUGO, name ## _show, NULL);
Signed-off-by: Harshit Jain <harshitjain6751@gmail.com>
---
drivers/input/touchscreen/ad7877.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/ad7877.c b/drivers/input/touchscreen/ad7877.c
index 5cfe477ec992..4b4a9d84195e 100644
--- a/drivers/input/touchscreen/ad7877.c
+++ b/drivers/input/touchscreen/ad7877.c
@@ -461,7 +461,7 @@ name ## _show(struct device *dev, struct device_attribute *attr, char *buf) \
return v; \
return sprintf(buf, "%u\n", (unsigned) v); \
} \
-static DEVICE_ATTR(name, S_IRUGO, name ## _show, NULL);
+static DEVICE_ATTR(name, 0444, name ## _show, NULL);
SHOW(aux1)
SHOW(aux2)
--
2.17.1
^ permalink raw reply related
* Re: [f2fs-dev] [PATCH v2] f2fs: fix to do sanity check with inline flags
From: Chao Yu @ 2018-07-23 13:18 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel
In-Reply-To: <20180723131708.GA23437@jaegeuk-macbookpro.roam.corp.google.com>
On 2018/7/23 21:17, Jaegeuk Kim wrote:
> On 07/15, Chao Yu wrote:
>> Hi Jaegeuk,
>>
>> Could you try this v2 to see whether it can solve the error injection problem?
>
> That doesn't work still.
Any dmesg there?
Thanks,
>
> Thanks,
>
>>
>> Thanks,
>>
>> On 2018/6/29 0:19, Chao Yu wrote:
>>> From: Chao Yu <yuchao0@huawei.com>
>>>
>>> https://bugzilla.kernel.org/show_bug.cgi?id=200221
>>>
>>> - Overview
>>> BUG() in clear_inode() when mounting and un-mounting a corrupted f2fs image
>>>
>>> - Reproduce
>>>
>>> - Kernel message
>>> [ 538.601448] F2FS-fs (loop0): Invalid segment/section count (31, 24 x 1376257)
>>> [ 538.601458] F2FS-fs (loop0): Can't find valid F2FS filesystem in 2th superblock
>>> [ 538.724091] F2FS-fs (loop0): Try to recover 2th superblock, ret: 0
>>> [ 538.724102] F2FS-fs (loop0): Mounted with checkpoint version = 2
>>> [ 540.970834] ------------[ cut here ]------------
>>> [ 540.970838] kernel BUG at fs/inode.c:512!
>>> [ 540.971750] invalid opcode: 0000 [#1] SMP KASAN PTI
>>> [ 540.972755] CPU: 1 PID: 1305 Comm: umount Not tainted 4.18.0-rc1+ #4
>>> [ 540.974034] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
>>> [ 540.982913] RIP: 0010:clear_inode+0xc0/0xd0
>>> [ 540.983774] Code: 8d a3 30 01 00 00 4c 89 e7 e8 1c ec f8 ff 48 8b 83 30 01 00 00 49 39 c4 75 1a 48 c7 83 a0 00 00 00 60 00 00 00 5b 41 5c 5d c3 <0f> 0b 0f 0b 0f 0b 0f 0b 0f 0b 0f 0b 0f 1f 40 00 66 66 66 66 90 55
>>> [ 540.987570] RSP: 0018:ffff8801e34a7b70 EFLAGS: 00010002
>>> [ 540.988636] RAX: 0000000000000000 RBX: ffff8801e9b744e8 RCX: ffffffffb840eb3a
>>> [ 540.990063] RDX: dffffc0000000000 RSI: 0000000000000004 RDI: ffff8801e9b746b8
>>> [ 540.991499] RBP: ffff8801e34a7b80 R08: ffffed003d36e8ce R09: ffffed003d36e8ce
>>> [ 540.992923] R10: 0000000000000001 R11: ffffed003d36e8cd R12: ffff8801e9b74668
>>> [ 540.994360] R13: ffff8801e9b74760 R14: ffff8801e9b74528 R15: ffff8801e9b74530
>>> [ 540.995786] FS: 00007f4662bdf840(0000) GS:ffff8801f6f00000(0000) knlGS:0000000000000000
>>> [ 540.997403] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [ 540.998571] CR2: 000000000175c568 CR3: 00000001dcfe6000 CR4: 00000000000006e0
>>> [ 541.000015] Call Trace:
>>> [ 541.000554] f2fs_evict_inode+0x253/0x630
>>> [ 541.001381] evict+0x16f/0x290
>>> [ 541.002015] iput+0x280/0x300
>>> [ 541.002654] dentry_unlink_inode+0x165/0x1e0
>>> [ 541.003528] __dentry_kill+0x16a/0x260
>>> [ 541.004300] dentry_kill+0x70/0x250
>>> [ 541.005018] dput+0x154/0x1d0
>>> [ 541.005635] do_one_tree+0x34/0x40
>>> [ 541.006354] shrink_dcache_for_umount+0x3f/0xa0
>>> [ 541.007285] generic_shutdown_super+0x43/0x1c0
>>> [ 541.008192] kill_block_super+0x52/0x80
>>> [ 541.008978] kill_f2fs_super+0x62/0x70
>>> [ 541.009750] deactivate_locked_super+0x6f/0xa0
>>> [ 541.010664] deactivate_super+0x5e/0x80
>>> [ 541.011450] cleanup_mnt+0x61/0xa0
>>> [ 541.012151] __cleanup_mnt+0x12/0x20
>>> [ 541.012893] task_work_run+0xc8/0xf0
>>> [ 541.013635] exit_to_usermode_loop+0x125/0x130
>>> [ 541.014555] do_syscall_64+0x138/0x170
>>> [ 541.015340] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>> [ 541.016375] RIP: 0033:0x7f46624bf487
>>> [ 541.017104] Code: 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 31 f6 e9 09 00 00 00 66 0f 1f 84 00 00 00 00 00 b8 a6 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d e1 c9 2b 00 f7 d8 64 89 01 48
>>> [ 541.020923] RSP: 002b:00007fff5e12e9a8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a6
>>> [ 541.022452] RAX: 0000000000000000 RBX: 0000000001753030 RCX: 00007f46624bf487
>>> [ 541.023885] RDX: 0000000000000001 RSI: 0000000000000000 RDI: 000000000175a1e0
>>> [ 541.025318] RBP: 000000000175a1e0 R08: 0000000000000000 R09: 0000000000000014
>>> [ 541.026755] R10: 00000000000006b2 R11: 0000000000000246 R12: 00007f46629c883c
>>> [ 541.028186] R13: 0000000000000000 R14: 0000000001753210 R15: 00007fff5e12ec30
>>> [ 541.029626] Modules linked in: snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core snd_pcm snd_timer snd mac_hid i2c_piix4 soundcore ib_iser rdma_cm iw_cm ib_cm ib_core iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx raid1 raid0 multipath linear 8139too crct10dif_pclmul crc32_pclmul qxl drm_kms_helper syscopyarea aesni_intel sysfillrect sysimgblt fb_sys_fops ttm drm aes_x86_64 crypto_simd cryptd 8139cp glue_helper mii pata_acpi floppy
>>> [ 541.039445] ---[ end trace 4ce02f25ff7d3df5 ]---
>>> [ 541.040392] RIP: 0010:clear_inode+0xc0/0xd0
>>> [ 541.041240] Code: 8d a3 30 01 00 00 4c 89 e7 e8 1c ec f8 ff 48 8b 83 30 01 00 00 49 39 c4 75 1a 48 c7 83 a0 00 00 00 60 00 00 00 5b 41 5c 5d c3 <0f> 0b 0f 0b 0f 0b 0f 0b 0f 0b 0f 0b 0f 1f 40 00 66 66 66 66 90 55
>>> [ 541.045042] RSP: 0018:ffff8801e34a7b70 EFLAGS: 00010002
>>> [ 541.046099] RAX: 0000000000000000 RBX: ffff8801e9b744e8 RCX: ffffffffb840eb3a
>>> [ 541.047537] RDX: dffffc0000000000 RSI: 0000000000000004 RDI: ffff8801e9b746b8
>>> [ 541.048965] RBP: ffff8801e34a7b80 R08: ffffed003d36e8ce R09: ffffed003d36e8ce
>>> [ 541.050402] R10: 0000000000000001 R11: ffffed003d36e8cd R12: ffff8801e9b74668
>>> [ 541.051832] R13: ffff8801e9b74760 R14: ffff8801e9b74528 R15: ffff8801e9b74530
>>> [ 541.053263] FS: 00007f4662bdf840(0000) GS:ffff8801f6f00000(0000) knlGS:0000000000000000
>>> [ 541.054891] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [ 541.056039] CR2: 000000000175c568 CR3: 00000001dcfe6000 CR4: 00000000000006e0
>>> [ 541.058506] ==================================================================
>>> [ 541.059991] BUG: KASAN: stack-out-of-bounds in update_stack_state+0x38c/0x3e0
>>> [ 541.061513] Read of size 8 at addr ffff8801e34a7970 by task umount/1305
>>>
>>> [ 541.063302] CPU: 1 PID: 1305 Comm: umount Tainted: G D 4.18.0-rc1+ #4
>>> [ 541.064838] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
>>> [ 541.066778] Call Trace:
>>> [ 541.067294] dump_stack+0x7b/0xb5
>>> [ 541.067986] print_address_description+0x70/0x290
>>> [ 541.068941] kasan_report+0x291/0x390
>>> [ 541.069692] ? update_stack_state+0x38c/0x3e0
>>> [ 541.070598] __asan_load8+0x54/0x90
>>> [ 541.071315] update_stack_state+0x38c/0x3e0
>>> [ 541.072172] ? __read_once_size_nocheck.constprop.7+0x20/0x20
>>> [ 541.073340] ? vprintk_func+0x27/0x60
>>> [ 541.074096] ? printk+0xa3/0xd3
>>> [ 541.074762] ? __save_stack_trace+0x5e/0x100
>>> [ 541.075634] unwind_next_frame.part.5+0x18e/0x490
>>> [ 541.076594] ? unwind_dump+0x290/0x290
>>> [ 541.077368] ? __show_regs+0x2c4/0x330
>>> [ 541.078142] __unwind_start+0x106/0x190
>>> [ 541.085422] __save_stack_trace+0x5e/0x100
>>> [ 541.086268] ? __save_stack_trace+0x5e/0x100
>>> [ 541.087161] ? unlink_anon_vmas+0xba/0x2c0
>>> [ 541.087997] save_stack_trace+0x1f/0x30
>>> [ 541.088782] save_stack+0x46/0xd0
>>> [ 541.089475] ? __alloc_pages_slowpath+0x1420/0x1420
>>> [ 541.090477] ? flush_tlb_mm_range+0x15e/0x220
>>> [ 541.091364] ? __dec_node_state+0x24/0xb0
>>> [ 541.092180] ? lock_page_memcg+0x85/0xf0
>>> [ 541.092979] ? unlock_page_memcg+0x16/0x80
>>> [ 541.093812] ? page_remove_rmap+0x198/0x520
>>> [ 541.094674] ? mark_page_accessed+0x133/0x200
>>> [ 541.095559] ? _cond_resched+0x1a/0x50
>>> [ 541.096326] ? unmap_page_range+0xcd4/0xe50
>>> [ 541.097179] ? rb_next+0x58/0x80
>>> [ 541.097845] ? rb_next+0x58/0x80
>>> [ 541.098518] __kasan_slab_free+0x13c/0x1a0
>>> [ 541.099352] ? unlink_anon_vmas+0xba/0x2c0
>>> [ 541.100184] kasan_slab_free+0xe/0x10
>>> [ 541.100934] kmem_cache_free+0x89/0x1e0
>>> [ 541.101724] unlink_anon_vmas+0xba/0x2c0
>>> [ 541.102534] free_pgtables+0x101/0x1b0
>>> [ 541.103299] exit_mmap+0x146/0x2a0
>>> [ 541.103996] ? __ia32_sys_munmap+0x50/0x50
>>> [ 541.104829] ? kasan_check_read+0x11/0x20
>>> [ 541.105649] ? mm_update_next_owner+0x322/0x380
>>> [ 541.106578] mmput+0x8b/0x1d0
>>> [ 541.107191] do_exit+0x43a/0x1390
>>> [ 541.107876] ? mm_update_next_owner+0x380/0x380
>>> [ 541.108791] ? deactivate_super+0x5e/0x80
>>> [ 541.109610] ? cleanup_mnt+0x61/0xa0
>>> [ 541.110351] ? __cleanup_mnt+0x12/0x20
>>> [ 541.111115] ? task_work_run+0xc8/0xf0
>>> [ 541.111879] ? exit_to_usermode_loop+0x125/0x130
>>> [ 541.112817] rewind_stack_do_exit+0x17/0x20
>>> [ 541.113666] RIP: 0033:0x7f46624bf487
>>> [ 541.114404] Code: Bad RIP value.
>>> [ 541.115094] RSP: 002b:00007fff5e12e9a8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a6
>>> [ 541.116605] RAX: 0000000000000000 RBX: 0000000001753030 RCX: 00007f46624bf487
>>> [ 541.118034] RDX: 0000000000000001 RSI: 0000000000000000 RDI: 000000000175a1e0
>>> [ 541.119472] RBP: 000000000175a1e0 R08: 0000000000000000 R09: 0000000000000014
>>> [ 541.120890] R10: 00000000000006b2 R11: 0000000000000246 R12: 00007f46629c883c
>>> [ 541.122321] R13: 0000000000000000 R14: 0000000001753210 R15: 00007fff5e12ec30
>>>
>>> [ 541.124061] The buggy address belongs to the page:
>>> [ 541.125042] page:ffffea00078d29c0 count:0 mapcount:0 mapping:0000000000000000 index:0x0
>>> [ 541.126651] flags: 0x2ffff0000000000()
>>> [ 541.127418] raw: 02ffff0000000000 dead000000000100 dead000000000200 0000000000000000
>>> [ 541.128963] raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
>>> [ 541.130516] page dumped because: kasan: bad access detected
>>>
>>> [ 541.131954] Memory state around the buggy address:
>>> [ 541.132924] ffff8801e34a7800: 00 f1 f1 f1 f1 00 f4 f4 f4 f3 f3 f3 f3 00 00 00
>>> [ 541.134378] ffff8801e34a7880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> [ 541.135814] >ffff8801e34a7900: 00 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1
>>> [ 541.137253] ^
>>> [ 541.138637] ffff8801e34a7980: f1 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> [ 541.140075] ffff8801e34a7a00: 00 00 00 00 00 00 00 00 f3 00 00 00 00 00 00 00
>>> [ 541.141509] ==================================================================
>>>
>>> - Location
>>> https://elixir.bootlin.com/linux/v4.18-rc1/source/fs/inode.c#L512
>>> BUG_ON(inode->i_data.nrpages);
>>>
>>> The root cause is root directory inode is corrupted, it has both
>>> inline_data and inline_dentry flag, and its nlink is zero, so in
>>> ->evict(), after dropping all page cache, it grabs page #0 for inline
>>> data truncation, result in panic in later clear_inode() where we will
>>> check inode->i_data.nrpages value.
>>>
>>> This patch adds inline flags check in sanity_check_inode, in addition,
>>> do sanity check with root inode's nlink.
>>>
>>> Reported-by Wen Xu <wen.xu@gatech.edu>
>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>> ---
>>> v2:
>>> - fix wrong condition of inode type.
>>> fs/f2fs/inode.c | 20 ++++++++++++++++++++
>>> fs/f2fs/super.c | 3 ++-
>>> 2 files changed, 22 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
>>> index 24d9c93941d4..cd183c135e3d 100644
>>> --- a/fs/f2fs/inode.c
>>> +++ b/fs/f2fs/inode.c
>>> @@ -229,6 +229,26 @@ static bool sanity_check_inode(struct inode *inode)
>>> return false;
>>> }
>>> }
>>> +
>>> + if (f2fs_has_inline_data(inode) &&
>>> + (!S_ISREG(inode->i_mode) && !S_ISLNK(inode->i_mode))) {
>>> + set_sbi_flag(sbi, SBI_NEED_FSCK);
>>> + f2fs_msg(sbi->sb, KERN_WARNING,
>>> + "%s: inode (ino=%lx, mode=%u) should not have "
>>> + "inline_data, run fsck to fix",
>>> + __func__, inode->i_ino, inode->i_mode);
>>> + return false;
>>> + }
>>> +
>>> + if (f2fs_has_inline_dentry(inode) && !S_ISDIR(inode->i_mode)) {
>>> + set_sbi_flag(sbi, SBI_NEED_FSCK);
>>> + f2fs_msg(sbi->sb, KERN_WARNING,
>>> + "%s: inode (ino=%lx, mode=%u) should not have "
>>> + "inline_dentry, run fsck to fix",
>>> + __func__, inode->i_ino, inode->i_mode);
>>> + return false;
>>> + }
>>> +
>>> return true;
>>> }
>>>
>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>> index a64fee9137dc..a61dd0cf6cc5 100644
>>> --- a/fs/f2fs/super.c
>>> +++ b/fs/f2fs/super.c
>>> @@ -2933,7 +2933,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>>> err = PTR_ERR(root);
>>> goto free_stats;
>>> }
>>> - if (!S_ISDIR(root->i_mode) || !root->i_blocks || !root->i_size) {
>>> + if (!S_ISDIR(root->i_mode) || !root->i_blocks ||
>>> + !root->i_size || !root->i_nlink) {
>>> iput(root);
>>> err = -EINVAL;
>>> goto free_stats;
>>>
^ permalink raw reply
* [Buildroot] [PATCH] vim: install /bin/vi as a relative symlink
From: Thomas Petazzoni @ 2018-07-23 13:17 UTC (permalink / raw)
To: buildroot
In-Reply-To: <1840086717.4674313.1532086956701.JavaMail.zimbra@datacom.com.br>
Hello,
On Fri, 20 Jul 2018 08:42:36 -0300 (BRT), Carlos Santos wrote:
> > But if we allow dangling non-relative symlinks I don't think we want to
> > add this complexity to the vim package, or any other package that
> > installs similar relative symlinks.
>
> The complexity is a consequence of the BR2_ROOTFS_MERGED_USR stuff.
> We already need to deal with such situations in six packages. I'd
> rather declare merged /usr as mandatory and send all those ifeq's
> down the tubes.
Perhaps we could introduce one or two helper functions that hide what
BR2_ROOTFS_MERGED_USR is doing. I thought a few minutes about this and
couldn't find immediately a good semantic/naming, but perhaps this is
a direction that could be investigated ?
Best regards,
Thomas Petazzoni
--
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH 1/6] ubi: Introduce vol_ignored()
From: Greg KH @ 2018-07-23 12:16 UTC (permalink / raw)
To: Richard Weinberger; +Cc: stable
In-Reply-To: <20180723121014.30778-2-richard@nod.at>
On Mon, Jul 23, 2018 at 02:10:09PM +0200, Richard Weinberger wrote:
> This makes the logic more easy to follow.
>
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
> drivers/mtd/ubi/attach.c | 24 ++++++++++++++++++------
> drivers/mtd/ubi/ubi.h | 15 +++++++++++++++
> 2 files changed, 33 insertions(+), 6 deletions(-)
Not having the commit id in the patch makes it a bit harder here. Next
time can you add it so I don't have to keep referring back to patch 0/6?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v3 1/2] f2fs: fix to avoid broken of dnode block list
From: Chao Yu @ 2018-07-23 13:17 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel, linux-kernel
In-Reply-To: <20180723122535.GA19644@jaegeuk-macbookpro.roam.corp.google.com>
On 2018/7/23 20:25, Jaegeuk Kim wrote:
> On 07/20, Chao Yu wrote:
>> Hi Jaegeuk,
>>
>> On 2018/7/15 9:30, Chao Yu wrote:
>>> On 2018/7/14 13:44, Jaegeuk Kim wrote:
>>>> On 07/08, Chao Yu wrote:
>>>>> From: Chao Yu <yuchao0@huawei.com>
>>>>>
>>>>> f2fs recovery flow is relying on dnode block link list, it means fsynced
>>>>> file recovery depends on previous dnode's persistence in the list, so
>>>>> during fsync() we should wait on all regular inode's dnode writebacked
>>>>> before issuing flush.
>>>>>
>>>>> By this way, we can avoid dnode block list being broken by out-of-order
>>>>> IO submission due to IO scheduler or driver.
>>>>
>>>> Hi Chao,
>>>>
>>>> Just in case, can we measure some performance numbers with this?
>>>
>>> Hi Jaegeuk,
>>>
>>> OK, let me do some tests on this.
>>
>> Sheng Yong helps to do the test with this patch:
>>
>> # Microbenchmark
>> Target:/data (f2fs, -)
>> 64MB / 32768KB / 4KB / 8
>>
>> # SQLite benchmark
>> 1 / PERSIST / Index
>>
>> Base:
>> SEQ-RD(MB/s) SEQ-WR(MB/s) RND-RD(IOPS) RND-WR(IOPS) Insert(TPS) Update(TPS) Delete(TPS)
>> 1 867.82 204.15 41440.03 41370.54 680.8 1025.94 1031.08
>> 2 871.87 205.87 41370.3 40275.2 791.14 1065.84 1101.7
>> 3 866.52 205.69 41795.67 40596.16 694.69 1037.16 1031.48
>> Avg 868.7366667 205.2366667 41535.33333 40747.3 722.21 1042.98 1054.753333
>>
>> After:
>> SEQ-RD(MB/s) SEQ-WR(MB/s) RND-RD(IOPS) RND-WR(IOPS) Insert(TPS) Update(TPS) Delete(TPS)
>> 1 798.81 202.5 41143 40613.87 602.71 838.08 913.83
>> 2 805.79 206.47 40297.2 41291.46 604.44 840.75 924.27
>> 3 814.83 206.17 41209.57 40453.62 602.85 834.66 927.91
>> Avg 806.4766667 205.0466667 40883.25667 40786.31667 603.3333333 837.83 922.0033333
>>
>> Orig/Patched:
>> 0.928332713 0.999074239 0.984300676 1.000957528 0.835398753 0.803303994 0.874141189
>>
>> It looks like atomic write will suffer performance regression.
>>
>> I suspect that the criminal is that we forcing to wait all dnode being in
>> storage cache before we issue PREFLUSH+FUA.
>>
>> BTW, will commit ("f2fs: don't need to wait for node writes for atomic write")
>> cause the problem: we will lose data of last transaction after SPO, even if
>> atomic write return no error:
>
> Practically, I don't see db corruption becase of this. We can excuse to lose
> the last transaction.
So, let me add these numbers in log of the patch, and change to skip waiting
node page for atomic write.
Thanks,
>
> Thanks,
>
>>
>> - atomic_open();
>> - write() P1, P2, P3;
>> - atomic_commit();
>> - writeback data: P1, P2, P3;
>> - writeback node: N1, N2, N3; <--- If N1, N2 is not writebacked, N3 with fsync_mark is
>> writebacked, In SPOR, we won't find N3 since node chain is broken, turns out that losing
>> last transaction.
>> - preflush + fua;
>> - power-cut
>>
>> Thanks,
>>
>>>
>>> Thanks,
>>>
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>>>> ---
>>>>> v3:
>>>>> - add a list to link all writebacked dnodes, let fsync() only wait on
>>>>> necessary dnode.
>>>>> fs/f2fs/checkpoint.c | 2 +
>>>>> fs/f2fs/data.c | 2 +
>>>>> fs/f2fs/f2fs.h | 21 +++++++-
>>>>> fs/f2fs/file.c | 20 +++----
>>>>> fs/f2fs/node.c | 148 +++++++++++++++++++++++++++++++++++++++++----------
>>>>> fs/f2fs/super.c | 4 ++
>>>>> 6 files changed, 153 insertions(+), 44 deletions(-)
>>>>>
>>>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>>>>> index 8b698bd54490..d5e60d76362e 100644
>>>>> --- a/fs/f2fs/checkpoint.c
>>>>> +++ b/fs/f2fs/checkpoint.c
>>>>> @@ -1379,6 +1379,8 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>>>>
>>>>> f2fs_release_ino_entry(sbi, false);
>>>>>
>>>>> + f2fs_reset_fsync_node_info(sbi);
>>>>> +
>>>>> if (unlikely(f2fs_cp_error(sbi)))
>>>>> return -EIO;
>>>>>
>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>> index 70813a4dda3e..afe76d87575c 100644
>>>>> --- a/fs/f2fs/data.c
>>>>> +++ b/fs/f2fs/data.c
>>>>> @@ -176,6 +176,8 @@ static void f2fs_write_end_io(struct bio *bio)
>>>>> page->index != nid_of_node(page));
>>>>>
>>>>> dec_page_count(sbi, WB_DATA_TYPE(page));
>>>>> + if (f2fs_in_warm_node_list(sbi, page))
>>>>> + f2fs_del_fsync_node_entry(sbi, page);
>>>>> clear_cold_data(page);
>>>>> end_page_writeback(page);
>>>>> }
>>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>>> index c8c865fa8450..bf5f7a336ace 100644
>>>>> --- a/fs/f2fs/f2fs.h
>>>>> +++ b/fs/f2fs/f2fs.h
>>>>> @@ -228,6 +228,12 @@ struct inode_entry {
>>>>> struct inode *inode; /* vfs inode pointer */
>>>>> };
>>>>>
>>>>> +struct fsync_node_entry {
>>>>> + struct list_head list; /* list head */
>>>>> + struct page *page; /* warm node page pointer */
>>>>> + unsigned int seq_id; /* sequence id */
>>>>> +};
>>>>> +
>>>>> /* for the bitmap indicate blocks to be discarded */
>>>>> struct discard_entry {
>>>>> struct list_head list; /* list head */
>>>>> @@ -1152,6 +1158,11 @@ struct f2fs_sb_info {
>>>>>
>>>>> struct inode_management im[MAX_INO_ENTRY]; /* manage inode cache */
>>>>>
>>>>> + spinlock_t fsync_node_lock; /* for node entry lock */
>>>>> + struct list_head fsync_node_list; /* node list head */
>>>>> + unsigned int fsync_seg_id; /* sequence id */
>>>>> + unsigned int fsync_node_num; /* number of node entries */
>>>>> +
>>>>> /* for orphan inode, use 0'th array */
>>>>> unsigned int max_orphans; /* max orphan inodes */
>>>>>
>>>>> @@ -2816,6 +2827,10 @@ struct node_info;
>>>>>
>>>>> int f2fs_check_nid_range(struct f2fs_sb_info *sbi, nid_t nid);
>>>>> bool f2fs_available_free_memory(struct f2fs_sb_info *sbi, int type);
>>>>> +bool f2fs_in_warm_node_list(struct f2fs_sb_info *sbi, struct page *page);
>>>>> +void f2fs_init_fsync_node_info(struct f2fs_sb_info *sbi);
>>>>> +void f2fs_del_fsync_node_entry(struct f2fs_sb_info *sbi, struct page *page);
>>>>> +void f2fs_reset_fsync_node_info(struct f2fs_sb_info *sbi);
>>>>> int f2fs_need_dentry_mark(struct f2fs_sb_info *sbi, nid_t nid);
>>>>> bool f2fs_is_checkpointed_node(struct f2fs_sb_info *sbi, nid_t nid);
>>>>> bool f2fs_need_inode_block_update(struct f2fs_sb_info *sbi, nid_t ino);
>>>>> @@ -2825,7 +2840,8 @@ pgoff_t f2fs_get_next_page_offset(struct dnode_of_data *dn, pgoff_t pgofs);
>>>>> int f2fs_get_dnode_of_data(struct dnode_of_data *dn, pgoff_t index, int mode);
>>>>> int f2fs_truncate_inode_blocks(struct inode *inode, pgoff_t from);
>>>>> int f2fs_truncate_xattr_node(struct inode *inode);
>>>>> -int f2fs_wait_on_node_pages_writeback(struct f2fs_sb_info *sbi, nid_t ino);
>>>>> +int f2fs_wait_on_node_pages_writeback(struct f2fs_sb_info *sbi,
>>>>> + unsigned int seq_id);
>>>>> int f2fs_remove_inode_page(struct inode *inode);
>>>>> struct page *f2fs_new_inode_page(struct inode *inode);
>>>>> struct page *f2fs_new_node_page(struct dnode_of_data *dn, unsigned int ofs);
>>>>> @@ -2834,7 +2850,8 @@ struct page *f2fs_get_node_page(struct f2fs_sb_info *sbi, pgoff_t nid);
>>>>> struct page *f2fs_get_node_page_ra(struct page *parent, int start);
>>>>> void f2fs_move_node_page(struct page *node_page, int gc_type);
>>>>> int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode,
>>>>> - struct writeback_control *wbc, bool atomic);
>>>>> + struct writeback_control *wbc, bool atomic,
>>>>> + unsigned int *seq_id);
>>>>> int f2fs_sync_node_pages(struct f2fs_sb_info *sbi,
>>>>> struct writeback_control *wbc,
>>>>> bool do_balance, enum iostat_type io_type);
>>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>>> index 5e29d4053748..ddea2bfd4042 100644
>>>>> --- a/fs/f2fs/file.c
>>>>> +++ b/fs/f2fs/file.c
>>>>> @@ -213,6 +213,7 @@ static int f2fs_do_sync_file(struct file *file, loff_t start, loff_t end,
>>>>> .nr_to_write = LONG_MAX,
>>>>> .for_reclaim = 0,
>>>>> };
>>>>> + unsigned int seq_id = 0;
>>>>>
>>>>> if (unlikely(f2fs_readonly(inode->i_sb)))
>>>>> return 0;
>>>>> @@ -275,7 +276,7 @@ static int f2fs_do_sync_file(struct file *file, loff_t start, loff_t end,
>>>>> }
>>>>> sync_nodes:
>>>>> atomic_inc(&sbi->wb_sync_req[NODE]);
>>>>> - ret = f2fs_fsync_node_pages(sbi, inode, &wbc, atomic);
>>>>> + ret = f2fs_fsync_node_pages(sbi, inode, &wbc, atomic, &seq_id);
>>>>> atomic_dec(&sbi->wb_sync_req[NODE]);
>>>>> if (ret)
>>>>> goto out;
>>>>> @@ -292,19 +293,10 @@ static int f2fs_do_sync_file(struct file *file, loff_t start, loff_t end,
>>>>> goto sync_nodes;
>>>>> }
>>>>>
>>>>> - /*
>>>>> - * If it's atomic_write, it's just fine to keep write ordering. So
>>>>> - * here we don't need to wait for node write completion, since we use
>>>>> - * node chain which serializes node blocks. If one of node writes are
>>>>> - * reordered, we can see simply broken chain, resulting in stopping
>>>>> - * roll-forward recovery. It means we'll recover all or none node blocks
>>>>> - * given fsync mark.
>>>>> - */
>>>>> - if (!atomic) {
>>>>> - ret = f2fs_wait_on_node_pages_writeback(sbi, ino);
>>>>> - if (ret)
>>>>> - goto out;
>>>>> - }
>>>>> +
>>>>> + ret = f2fs_wait_on_node_pages_writeback(sbi, seq_id);
>>>>> + if (ret)
>>>>> + goto out;
>>>>>
>>>>> /* once recovery info is written, don't need to tack this */
>>>>> f2fs_remove_ino_entry(sbi, ino, APPEND_INO);
>>>>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>>>>> index 1e30bc305243..31dc372c56a0 100644
>>>>> --- a/fs/f2fs/node.c
>>>>> +++ b/fs/f2fs/node.c
>>>>> @@ -28,6 +28,7 @@
>>>>> static struct kmem_cache *nat_entry_slab;
>>>>> static struct kmem_cache *free_nid_slab;
>>>>> static struct kmem_cache *nat_entry_set_slab;
>>>>> +static struct kmem_cache *fsync_node_entry_slab;
>>>>>
>>>>> /*
>>>>> * Check whether the given nid is within node id range.
>>>>> @@ -267,6 +268,72 @@ static unsigned int __gang_lookup_nat_set(struct f2fs_nm_info *nm_i,
>>>>> start, nr);
>>>>> }
>>>>>
>>>>> +bool f2fs_in_warm_node_list(struct f2fs_sb_info *sbi, struct page *page)
>>>>> +{
>>>>> + return NODE_MAPPING(sbi) == page->mapping &&
>>>>> + IS_DNODE(page) && is_cold_node(page);
>>>>> +}
>>>>> +
>>>>> +void f2fs_init_fsync_node_info(struct f2fs_sb_info *sbi)
>>>>> +{
>>>>> + spin_lock_init(&sbi->fsync_node_lock);
>>>>> + INIT_LIST_HEAD(&sbi->fsync_node_list);
>>>>> + sbi->fsync_seg_id = 0;
>>>>> + sbi->fsync_node_num = 0;
>>>>> +}
>>>>> +
>>>>> +static unsigned int f2fs_add_fsync_node_entry(struct f2fs_sb_info *sbi,
>>>>> + struct page *page)
>>>>> +{
>>>>> + struct fsync_node_entry *fn;
>>>>> + unsigned long flags;
>>>>> + unsigned int seq_id;
>>>>> +
>>>>> + fn = f2fs_kmem_cache_alloc(fsync_node_entry_slab, GFP_NOFS);
>>>>> +
>>>>> + get_page(page);
>>>>> + fn->page = page;
>>>>> + INIT_LIST_HEAD(&fn->list);
>>>>> +
>>>>> + spin_lock_irqsave(&sbi->fsync_node_lock, flags);
>>>>> + list_add_tail(&fn->list, &sbi->fsync_node_list);
>>>>> + fn->seq_id = sbi->fsync_seg_id++;
>>>>> + seq_id = fn->seq_id;
>>>>> + sbi->fsync_node_num++;
>>>>> + spin_unlock_irqrestore(&sbi->fsync_node_lock, flags);
>>>>> +
>>>>> + return seq_id;
>>>>> +}
>>>>> +
>>>>> +void f2fs_del_fsync_node_entry(struct f2fs_sb_info *sbi, struct page *page)
>>>>> +{
>>>>> + struct fsync_node_entry *fn;
>>>>> + unsigned long flags;
>>>>> +
>>>>> + spin_lock_irqsave(&sbi->fsync_node_lock, flags);
>>>>> + list_for_each_entry(fn, &sbi->fsync_node_list, list) {
>>>>> + if (fn->page == page) {
>>>>> + list_del(&fn->list);
>>>>> + sbi->fsync_node_num--;
>>>>> + spin_unlock_irqrestore(&sbi->fsync_node_lock, flags);
>>>>> + kmem_cache_free(fsync_node_entry_slab, fn);
>>>>> + put_page(page);
>>>>> + return;
>>>>> + }
>>>>> + }
>>>>> + spin_unlock_irqrestore(&sbi->fsync_node_lock, flags);
>>>>> + f2fs_bug_on(sbi, 1);
>>>>> +}
>>>>> +
>>>>> +void f2fs_reset_fsync_node_info(struct f2fs_sb_info *sbi)
>>>>> +{
>>>>> + unsigned long flags;
>>>>> +
>>>>> + spin_lock_irqsave(&sbi->fsync_node_lock, flags);
>>>>> + sbi->fsync_node_num = 0;
>>>>> + spin_unlock_irqrestore(&sbi->fsync_node_lock, flags);
>>>>> +}
>>>>> +
>>>>> int f2fs_need_dentry_mark(struct f2fs_sb_info *sbi, nid_t nid)
>>>>> {
>>>>> struct f2fs_nm_info *nm_i = NM_I(sbi);
>>>>> @@ -1353,7 +1420,7 @@ static struct page *last_fsync_dnode(struct f2fs_sb_info *sbi, nid_t ino)
>>>>>
>>>>> static int __write_node_page(struct page *page, bool atomic, bool *submitted,
>>>>> struct writeback_control *wbc, bool do_balance,
>>>>> - enum iostat_type io_type)
>>>>> + enum iostat_type io_type, unsigned int *seq_id)
>>>>> {
>>>>> struct f2fs_sb_info *sbi = F2FS_P_SB(page);
>>>>> nid_t nid;
>>>>> @@ -1370,6 +1437,7 @@ static int __write_node_page(struct page *page, bool atomic, bool *submitted,
>>>>> .io_type = io_type,
>>>>> .io_wbc = wbc,
>>>>> };
>>>>> + unsigned int seq;
>>>>>
>>>>> trace_f2fs_writepage(page, NODE);
>>>>>
>>>>> @@ -1416,6 +1484,13 @@ static int __write_node_page(struct page *page, bool atomic, bool *submitted,
>>>>>
>>>>> set_page_writeback(page);
>>>>> ClearPageError(page);
>>>>> +
>>>>> + if (f2fs_in_warm_node_list(sbi, page)) {
>>>>> + seq = f2fs_add_fsync_node_entry(sbi, page);
>>>>> + if (seq_id)
>>>>> + *seq_id = seq;
>>>>> + }
>>>>> +
>>>>> fio.old_blkaddr = ni.blk_addr;
>>>>> f2fs_do_write_node_page(nid, &fio);
>>>>> set_node_addr(sbi, &ni, fio.new_blkaddr, is_fsync_dnode(page));
>>>>> @@ -1463,7 +1538,7 @@ void f2fs_move_node_page(struct page *node_page, int gc_type)
>>>>> goto out_page;
>>>>>
>>>>> if (__write_node_page(node_page, false, NULL,
>>>>> - &wbc, false, FS_GC_NODE_IO))
>>>>> + &wbc, false, FS_GC_NODE_IO, NULL))
>>>>> unlock_page(node_page);
>>>>> goto release_page;
>>>>> } else {
>>>>> @@ -1480,11 +1555,13 @@ void f2fs_move_node_page(struct page *node_page, int gc_type)
>>>>> static int f2fs_write_node_page(struct page *page,
>>>>> struct writeback_control *wbc)
>>>>> {
>>>>> - return __write_node_page(page, false, NULL, wbc, false, FS_NODE_IO);
>>>>> + return __write_node_page(page, false, NULL, wbc, false,
>>>>> + FS_NODE_IO, NULL);
>>>>> }
>>>>>
>>>>> int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode,
>>>>> - struct writeback_control *wbc, bool atomic)
>>>>> + struct writeback_control *wbc, bool atomic,
>>>>> + unsigned int *seq_id)
>>>>> {
>>>>> pgoff_t index;
>>>>> pgoff_t last_idx = ULONG_MAX;
>>>>> @@ -1565,7 +1642,7 @@ int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode,
>>>>> ret = __write_node_page(page, atomic &&
>>>>> page == last_page,
>>>>> &submitted, wbc, true,
>>>>> - FS_NODE_IO);
>>>>> + FS_NODE_IO, seq_id);
>>>>> if (ret) {
>>>>> unlock_page(page);
>>>>> f2fs_put_page(last_page, 0);
>>>>> @@ -1682,7 +1759,7 @@ int f2fs_sync_node_pages(struct f2fs_sb_info *sbi,
>>>>> set_dentry_mark(page, 0);
>>>>>
>>>>> ret = __write_node_page(page, false, &submitted,
>>>>> - wbc, do_balance, io_type);
>>>>> + wbc, do_balance, io_type, NULL);
>>>>> if (ret)
>>>>> unlock_page(page);
>>>>> else if (submitted)
>>>>> @@ -1713,35 +1790,42 @@ int f2fs_sync_node_pages(struct f2fs_sb_info *sbi,
>>>>> return ret;
>>>>> }
>>>>>
>>>>> -int f2fs_wait_on_node_pages_writeback(struct f2fs_sb_info *sbi, nid_t ino)
>>>>> +int f2fs_wait_on_node_pages_writeback(struct f2fs_sb_info *sbi,
>>>>> + unsigned int seq_id)
>>>>> {
>>>>> - pgoff_t index = 0;
>>>>> - struct pagevec pvec;
>>>>> - int ret2, ret = 0;
>>>>> - int nr_pages;
>>>>> + struct fsync_node_entry *fn;
>>>>> + struct page *page;
>>>>> + struct list_head *head = &sbi->fsync_node_list;
>>>>> + unsigned long flags;
>>>>> + unsigned int cur_seq_id = 0;
>>>>> + int ret = 0;
>>>>>
>>>>> - pagevec_init(&pvec);
>>>>> + while (seq_id && cur_seq_id < seq_id) {
>>>>> + spin_lock_irqsave(&sbi->fsync_node_lock, flags);
>>>>> + if (list_empty(head)) {
>>>>> + spin_unlock_irqrestore(&sbi->fsync_node_lock, flags);
>>>>> + break;
>>>>> + }
>>>>> + fn = list_first_entry(head, struct fsync_node_entry, list);
>>>>> + if (fn->seq_id > seq_id) {
>>>>> + spin_unlock_irqrestore(&sbi->fsync_node_lock, flags);
>>>>> + break;
>>>>> + }
>>>>> + cur_seq_id = fn->seq_id;
>>>>> + page = fn->page;
>>>>> + get_page(page);
>>>>> + spin_unlock_irqrestore(&sbi->fsync_node_lock, flags);
>>>>>
>>>>> - while ((nr_pages = pagevec_lookup_tag(&pvec, NODE_MAPPING(sbi), &index,
>>>>> - PAGECACHE_TAG_WRITEBACK))) {
>>>>> - int i;
>>>>> + f2fs_wait_on_page_writeback(page, NODE, true);
>>>>> + if (TestClearPageError(page))
>>>>> + ret = -EIO;
>>>>>
>>>>> - for (i = 0; i < nr_pages; i++) {
>>>>> - struct page *page = pvec.pages[i];
>>>>> + put_page(page);
>>>>>
>>>>> - if (ino && ino_of_node(page) == ino) {
>>>>> - f2fs_wait_on_page_writeback(page, NODE, true);
>>>>> - if (TestClearPageError(page))
>>>>> - ret = -EIO;
>>>>> - }
>>>>> - }
>>>>> - pagevec_release(&pvec);
>>>>> - cond_resched();
>>>>> + if (ret)
>>>>> + break;
>>>>> }
>>>>>
>>>>> - ret2 = filemap_check_errors(NODE_MAPPING(sbi));
>>>>> - if (!ret)
>>>>> - ret = ret2;
>>>>> return ret;
>>>>> }
>>>>>
>>>>> @@ -2939,8 +3023,15 @@ int __init f2fs_create_node_manager_caches(void)
>>>>> sizeof(struct nat_entry_set));
>>>>> if (!nat_entry_set_slab)
>>>>> goto destroy_free_nid;
>>>>> +
>>>>> + fsync_node_entry_slab = f2fs_kmem_cache_create("fsync_node_entry",
>>>>> + sizeof(struct fsync_node_entry));
>>>>> + if (!fsync_node_entry_slab)
>>>>> + goto destroy_nat_entry_set;
>>>>> return 0;
>>>>>
>>>>> +destroy_nat_entry_set:
>>>>> + kmem_cache_destroy(nat_entry_set_slab);
>>>>> destroy_free_nid:
>>>>> kmem_cache_destroy(free_nid_slab);
>>>>> destroy_nat_entry:
>>>>> @@ -2951,6 +3042,7 @@ int __init f2fs_create_node_manager_caches(void)
>>>>>
>>>>> void f2fs_destroy_node_manager_caches(void)
>>>>> {
>>>>> + kmem_cache_destroy(fsync_node_entry_slab);
>>>>> kmem_cache_destroy(nat_entry_set_slab);
>>>>> kmem_cache_destroy(free_nid_slab);
>>>>> kmem_cache_destroy(nat_entry_slab);
>>>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>>>> index 143ed321076e..34321932754d 100644
>>>>> --- a/fs/f2fs/super.c
>>>>> +++ b/fs/f2fs/super.c
>>>>> @@ -1023,6 +1023,8 @@ static void f2fs_put_super(struct super_block *sb)
>>>>> */
>>>>> f2fs_release_ino_entry(sbi, true);
>>>>>
>>>>> + f2fs_bug_on(sbi, sbi->fsync_node_num);
>>>>> +
>>>>> f2fs_leave_shrinker(sbi);
>>>>> mutex_unlock(&sbi->umount_mutex);
>>>>>
>>>>> @@ -2903,6 +2905,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>>>>>
>>>>> f2fs_init_ino_entry_info(sbi);
>>>>>
>>>>> + f2fs_init_fsync_node_info(sbi);
>>>>> +
>>>>> /* setup f2fs internal modules */
>>>>> err = f2fs_build_segment_manager(sbi);
>>>>> if (err) {
>>>>> --
>>>>> 2.16.2.17.g38e79b1fd
>>>
>>> .
>>>
^ permalink raw reply
* Re: [f2fs-dev] [PATCH v2] f2fs: fix to do sanity check with inline flags
From: Jaegeuk Kim @ 2018-07-23 13:17 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
In-Reply-To: <2931f2db-3a45-6641-8f4f-07e757b811d8@kernel.org>
On 07/15, Chao Yu wrote:
> Hi Jaegeuk,
>
> Could you try this v2 to see whether it can solve the error injection problem?
That doesn't work still.
Thanks,
>
> Thanks,
>
> On 2018/6/29 0:19, Chao Yu wrote:
> > From: Chao Yu <yuchao0@huawei.com>
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=200221
> >
> > - Overview
> > BUG() in clear_inode() when mounting and un-mounting a corrupted f2fs image
> >
> > - Reproduce
> >
> > - Kernel message
> > [ 538.601448] F2FS-fs (loop0): Invalid segment/section count (31, 24 x 1376257)
> > [ 538.601458] F2FS-fs (loop0): Can't find valid F2FS filesystem in 2th superblock
> > [ 538.724091] F2FS-fs (loop0): Try to recover 2th superblock, ret: 0
> > [ 538.724102] F2FS-fs (loop0): Mounted with checkpoint version = 2
> > [ 540.970834] ------------[ cut here ]------------
> > [ 540.970838] kernel BUG at fs/inode.c:512!
> > [ 540.971750] invalid opcode: 0000 [#1] SMP KASAN PTI
> > [ 540.972755] CPU: 1 PID: 1305 Comm: umount Not tainted 4.18.0-rc1+ #4
> > [ 540.974034] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
> > [ 540.982913] RIP: 0010:clear_inode+0xc0/0xd0
> > [ 540.983774] Code: 8d a3 30 01 00 00 4c 89 e7 e8 1c ec f8 ff 48 8b 83 30 01 00 00 49 39 c4 75 1a 48 c7 83 a0 00 00 00 60 00 00 00 5b 41 5c 5d c3 <0f> 0b 0f 0b 0f 0b 0f 0b 0f 0b 0f 0b 0f 1f 40 00 66 66 66 66 90 55
> > [ 540.987570] RSP: 0018:ffff8801e34a7b70 EFLAGS: 00010002
> > [ 540.988636] RAX: 0000000000000000 RBX: ffff8801e9b744e8 RCX: ffffffffb840eb3a
> > [ 540.990063] RDX: dffffc0000000000 RSI: 0000000000000004 RDI: ffff8801e9b746b8
> > [ 540.991499] RBP: ffff8801e34a7b80 R08: ffffed003d36e8ce R09: ffffed003d36e8ce
> > [ 540.992923] R10: 0000000000000001 R11: ffffed003d36e8cd R12: ffff8801e9b74668
> > [ 540.994360] R13: ffff8801e9b74760 R14: ffff8801e9b74528 R15: ffff8801e9b74530
> > [ 540.995786] FS: 00007f4662bdf840(0000) GS:ffff8801f6f00000(0000) knlGS:0000000000000000
> > [ 540.997403] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 540.998571] CR2: 000000000175c568 CR3: 00000001dcfe6000 CR4: 00000000000006e0
> > [ 541.000015] Call Trace:
> > [ 541.000554] f2fs_evict_inode+0x253/0x630
> > [ 541.001381] evict+0x16f/0x290
> > [ 541.002015] iput+0x280/0x300
> > [ 541.002654] dentry_unlink_inode+0x165/0x1e0
> > [ 541.003528] __dentry_kill+0x16a/0x260
> > [ 541.004300] dentry_kill+0x70/0x250
> > [ 541.005018] dput+0x154/0x1d0
> > [ 541.005635] do_one_tree+0x34/0x40
> > [ 541.006354] shrink_dcache_for_umount+0x3f/0xa0
> > [ 541.007285] generic_shutdown_super+0x43/0x1c0
> > [ 541.008192] kill_block_super+0x52/0x80
> > [ 541.008978] kill_f2fs_super+0x62/0x70
> > [ 541.009750] deactivate_locked_super+0x6f/0xa0
> > [ 541.010664] deactivate_super+0x5e/0x80
> > [ 541.011450] cleanup_mnt+0x61/0xa0
> > [ 541.012151] __cleanup_mnt+0x12/0x20
> > [ 541.012893] task_work_run+0xc8/0xf0
> > [ 541.013635] exit_to_usermode_loop+0x125/0x130
> > [ 541.014555] do_syscall_64+0x138/0x170
> > [ 541.015340] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > [ 541.016375] RIP: 0033:0x7f46624bf487
> > [ 541.017104] Code: 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 31 f6 e9 09 00 00 00 66 0f 1f 84 00 00 00 00 00 b8 a6 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d e1 c9 2b 00 f7 d8 64 89 01 48
> > [ 541.020923] RSP: 002b:00007fff5e12e9a8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a6
> > [ 541.022452] RAX: 0000000000000000 RBX: 0000000001753030 RCX: 00007f46624bf487
> > [ 541.023885] RDX: 0000000000000001 RSI: 0000000000000000 RDI: 000000000175a1e0
> > [ 541.025318] RBP: 000000000175a1e0 R08: 0000000000000000 R09: 0000000000000014
> > [ 541.026755] R10: 00000000000006b2 R11: 0000000000000246 R12: 00007f46629c883c
> > [ 541.028186] R13: 0000000000000000 R14: 0000000001753210 R15: 00007fff5e12ec30
> > [ 541.029626] Modules linked in: snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core snd_pcm snd_timer snd mac_hid i2c_piix4 soundcore ib_iser rdma_cm iw_cm ib_cm ib_core iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx raid1 raid0 multipath linear 8139too crct10dif_pclmul crc32_pclmul qxl drm_kms_helper syscopyarea aesni_intel sysfillrect sysimgblt fb_sys_fops ttm drm aes_x86_64 crypto_simd cryptd 8139cp glue_helper mii pata_acpi floppy
> > [ 541.039445] ---[ end trace 4ce02f25ff7d3df5 ]---
> > [ 541.040392] RIP: 0010:clear_inode+0xc0/0xd0
> > [ 541.041240] Code: 8d a3 30 01 00 00 4c 89 e7 e8 1c ec f8 ff 48 8b 83 30 01 00 00 49 39 c4 75 1a 48 c7 83 a0 00 00 00 60 00 00 00 5b 41 5c 5d c3 <0f> 0b 0f 0b 0f 0b 0f 0b 0f 0b 0f 0b 0f 1f 40 00 66 66 66 66 90 55
> > [ 541.045042] RSP: 0018:ffff8801e34a7b70 EFLAGS: 00010002
> > [ 541.046099] RAX: 0000000000000000 RBX: ffff8801e9b744e8 RCX: ffffffffb840eb3a
> > [ 541.047537] RDX: dffffc0000000000 RSI: 0000000000000004 RDI: ffff8801e9b746b8
> > [ 541.048965] RBP: ffff8801e34a7b80 R08: ffffed003d36e8ce R09: ffffed003d36e8ce
> > [ 541.050402] R10: 0000000000000001 R11: ffffed003d36e8cd R12: ffff8801e9b74668
> > [ 541.051832] R13: ffff8801e9b74760 R14: ffff8801e9b74528 R15: ffff8801e9b74530
> > [ 541.053263] FS: 00007f4662bdf840(0000) GS:ffff8801f6f00000(0000) knlGS:0000000000000000
> > [ 541.054891] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 541.056039] CR2: 000000000175c568 CR3: 00000001dcfe6000 CR4: 00000000000006e0
> > [ 541.058506] ==================================================================
> > [ 541.059991] BUG: KASAN: stack-out-of-bounds in update_stack_state+0x38c/0x3e0
> > [ 541.061513] Read of size 8 at addr ffff8801e34a7970 by task umount/1305
> >
> > [ 541.063302] CPU: 1 PID: 1305 Comm: umount Tainted: G D 4.18.0-rc1+ #4
> > [ 541.064838] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
> > [ 541.066778] Call Trace:
> > [ 541.067294] dump_stack+0x7b/0xb5
> > [ 541.067986] print_address_description+0x70/0x290
> > [ 541.068941] kasan_report+0x291/0x390
> > [ 541.069692] ? update_stack_state+0x38c/0x3e0
> > [ 541.070598] __asan_load8+0x54/0x90
> > [ 541.071315] update_stack_state+0x38c/0x3e0
> > [ 541.072172] ? __read_once_size_nocheck.constprop.7+0x20/0x20
> > [ 541.073340] ? vprintk_func+0x27/0x60
> > [ 541.074096] ? printk+0xa3/0xd3
> > [ 541.074762] ? __save_stack_trace+0x5e/0x100
> > [ 541.075634] unwind_next_frame.part.5+0x18e/0x490
> > [ 541.076594] ? unwind_dump+0x290/0x290
> > [ 541.077368] ? __show_regs+0x2c4/0x330
> > [ 541.078142] __unwind_start+0x106/0x190
> > [ 541.085422] __save_stack_trace+0x5e/0x100
> > [ 541.086268] ? __save_stack_trace+0x5e/0x100
> > [ 541.087161] ? unlink_anon_vmas+0xba/0x2c0
> > [ 541.087997] save_stack_trace+0x1f/0x30
> > [ 541.088782] save_stack+0x46/0xd0
> > [ 541.089475] ? __alloc_pages_slowpath+0x1420/0x1420
> > [ 541.090477] ? flush_tlb_mm_range+0x15e/0x220
> > [ 541.091364] ? __dec_node_state+0x24/0xb0
> > [ 541.092180] ? lock_page_memcg+0x85/0xf0
> > [ 541.092979] ? unlock_page_memcg+0x16/0x80
> > [ 541.093812] ? page_remove_rmap+0x198/0x520
> > [ 541.094674] ? mark_page_accessed+0x133/0x200
> > [ 541.095559] ? _cond_resched+0x1a/0x50
> > [ 541.096326] ? unmap_page_range+0xcd4/0xe50
> > [ 541.097179] ? rb_next+0x58/0x80
> > [ 541.097845] ? rb_next+0x58/0x80
> > [ 541.098518] __kasan_slab_free+0x13c/0x1a0
> > [ 541.099352] ? unlink_anon_vmas+0xba/0x2c0
> > [ 541.100184] kasan_slab_free+0xe/0x10
> > [ 541.100934] kmem_cache_free+0x89/0x1e0
> > [ 541.101724] unlink_anon_vmas+0xba/0x2c0
> > [ 541.102534] free_pgtables+0x101/0x1b0
> > [ 541.103299] exit_mmap+0x146/0x2a0
> > [ 541.103996] ? __ia32_sys_munmap+0x50/0x50
> > [ 541.104829] ? kasan_check_read+0x11/0x20
> > [ 541.105649] ? mm_update_next_owner+0x322/0x380
> > [ 541.106578] mmput+0x8b/0x1d0
> > [ 541.107191] do_exit+0x43a/0x1390
> > [ 541.107876] ? mm_update_next_owner+0x380/0x380
> > [ 541.108791] ? deactivate_super+0x5e/0x80
> > [ 541.109610] ? cleanup_mnt+0x61/0xa0
> > [ 541.110351] ? __cleanup_mnt+0x12/0x20
> > [ 541.111115] ? task_work_run+0xc8/0xf0
> > [ 541.111879] ? exit_to_usermode_loop+0x125/0x130
> > [ 541.112817] rewind_stack_do_exit+0x17/0x20
> > [ 541.113666] RIP: 0033:0x7f46624bf487
> > [ 541.114404] Code: Bad RIP value.
> > [ 541.115094] RSP: 002b:00007fff5e12e9a8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a6
> > [ 541.116605] RAX: 0000000000000000 RBX: 0000000001753030 RCX: 00007f46624bf487
> > [ 541.118034] RDX: 0000000000000001 RSI: 0000000000000000 RDI: 000000000175a1e0
> > [ 541.119472] RBP: 000000000175a1e0 R08: 0000000000000000 R09: 0000000000000014
> > [ 541.120890] R10: 00000000000006b2 R11: 0000000000000246 R12: 00007f46629c883c
> > [ 541.122321] R13: 0000000000000000 R14: 0000000001753210 R15: 00007fff5e12ec30
> >
> > [ 541.124061] The buggy address belongs to the page:
> > [ 541.125042] page:ffffea00078d29c0 count:0 mapcount:0 mapping:0000000000000000 index:0x0
> > [ 541.126651] flags: 0x2ffff0000000000()
> > [ 541.127418] raw: 02ffff0000000000 dead000000000100 dead000000000200 0000000000000000
> > [ 541.128963] raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
> > [ 541.130516] page dumped because: kasan: bad access detected
> >
> > [ 541.131954] Memory state around the buggy address:
> > [ 541.132924] ffff8801e34a7800: 00 f1 f1 f1 f1 00 f4 f4 f4 f3 f3 f3 f3 00 00 00
> > [ 541.134378] ffff8801e34a7880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > [ 541.135814] >ffff8801e34a7900: 00 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1
> > [ 541.137253] ^
> > [ 541.138637] ffff8801e34a7980: f1 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > [ 541.140075] ffff8801e34a7a00: 00 00 00 00 00 00 00 00 f3 00 00 00 00 00 00 00
> > [ 541.141509] ==================================================================
> >
> > - Location
> > https://elixir.bootlin.com/linux/v4.18-rc1/source/fs/inode.c#L512
> > BUG_ON(inode->i_data.nrpages);
> >
> > The root cause is root directory inode is corrupted, it has both
> > inline_data and inline_dentry flag, and its nlink is zero, so in
> > ->evict(), after dropping all page cache, it grabs page #0 for inline
> > data truncation, result in panic in later clear_inode() where we will
> > check inode->i_data.nrpages value.
> >
> > This patch adds inline flags check in sanity_check_inode, in addition,
> > do sanity check with root inode's nlink.
> >
> > Reported-by Wen Xu <wen.xu@gatech.edu>
> > Signed-off-by: Chao Yu <yuchao0@huawei.com>
> > ---
> > v2:
> > - fix wrong condition of inode type.
> > fs/f2fs/inode.c | 20 ++++++++++++++++++++
> > fs/f2fs/super.c | 3 ++-
> > 2 files changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> > index 24d9c93941d4..cd183c135e3d 100644
> > --- a/fs/f2fs/inode.c
> > +++ b/fs/f2fs/inode.c
> > @@ -229,6 +229,26 @@ static bool sanity_check_inode(struct inode *inode)
> > return false;
> > }
> > }
> > +
> > + if (f2fs_has_inline_data(inode) &&
> > + (!S_ISREG(inode->i_mode) && !S_ISLNK(inode->i_mode))) {
> > + set_sbi_flag(sbi, SBI_NEED_FSCK);
> > + f2fs_msg(sbi->sb, KERN_WARNING,
> > + "%s: inode (ino=%lx, mode=%u) should not have "
> > + "inline_data, run fsck to fix",
> > + __func__, inode->i_ino, inode->i_mode);
> > + return false;
> > + }
> > +
> > + if (f2fs_has_inline_dentry(inode) && !S_ISDIR(inode->i_mode)) {
> > + set_sbi_flag(sbi, SBI_NEED_FSCK);
> > + f2fs_msg(sbi->sb, KERN_WARNING,
> > + "%s: inode (ino=%lx, mode=%u) should not have "
> > + "inline_dentry, run fsck to fix",
> > + __func__, inode->i_ino, inode->i_mode);
> > + return false;
> > + }
> > +
> > return true;
> > }
> >
> > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > index a64fee9137dc..a61dd0cf6cc5 100644
> > --- a/fs/f2fs/super.c
> > +++ b/fs/f2fs/super.c
> > @@ -2933,7 +2933,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
> > err = PTR_ERR(root);
> > goto free_stats;
> > }
> > - if (!S_ISDIR(root->i_mode) || !root->i_blocks || !root->i_size) {
> > + if (!S_ISDIR(root->i_mode) || !root->i_blocks ||
> > + !root->i_size || !root->i_nlink) {
> > iput(root);
> > err = -EINVAL;
> > goto free_stats;
> >
^ permalink raw reply
* Re: [RFC PATCH ghak90 (was ghak32) V3 02/10] audit: log container info of syscalls
From: Paul Moore @ 2018-07-23 13:16 UTC (permalink / raw)
To: rgb
Cc: sgrubb, cgroups, containers, linux-api, linux-audit,
linux-fsdevel, linux-kernel, netdev, ebiederm, luto, carlos,
dhowells, viro, simo, Eric Paris, serge
In-Reply-To: <20180721202930.a7rypxc5rxi3hyiv@madcap2.tricolour.ca>
On Sat, Jul 21, 2018 at 4:32 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2018-07-20 18:13, Paul Moore wrote:
> > On Wed, Jun 6, 2018 at 1:00 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > Create a new audit record AUDIT_CONTAINER to document the audit
> > > container identifier of a process if it is present.
> > >
> > > Called from audit_log_exit(), syscalls are covered.
> > >
> > > A sample raw event:
> > > type=SYSCALL msg=audit(1519924845.499:257): arch=c000003e syscall=257 success=yes exit=3 a0=ffffff9c a1=56374e1cef30 a2=241 a3=1b6 items=2 ppid=606 pid=635 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=pts0 ses=3 comm="bash" exe="/usr/bin/bash" subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key="tmpcontainerid"
> > > type=CWD msg=audit(1519924845.499:257): cwd="/root"
> > > type=PATH msg=audit(1519924845.499:257): item=0 name="/tmp/" inode=13863 dev=00:27 mode=041777 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype= PARENT cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> > > type=PATH msg=audit(1519924845.499:257): item=1 name="/tmp/tmpcontainerid" inode=17729 dev=00:27 mode=0100644 ouid=0 ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=CREATE cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> > > type=PROCTITLE msg=audit(1519924845.499:257): proctitle=62617368002D6300736C65657020313B206563686F2074657374203E202F746D702F746D70636F6E7461696E65726964
> > > type=CONTAINER msg=audit(1519924845.499:257): op=task contid=123458
> > >
> > > See: https://github.com/linux-audit/audit-kernel/issues/90
> > > See: https://github.com/linux-audit/audit-userspace/issues/51
> > > See: https://github.com/linux-audit/audit-testsuite/issues/64
> > > See: https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Container-ID
> > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > ---
> > > include/linux/audit.h | 7 +++++++
> > > include/uapi/linux/audit.h | 1 +
> > > kernel/audit.c | 23 +++++++++++++++++++++++
> > > kernel/auditsc.c | 3 +++
> > > 4 files changed, 34 insertions(+)
> >
> > ...
> >
> > > --- a/include/uapi/linux/audit.h
> > > +++ b/include/uapi/linux/audit.h
> > > @@ -115,6 +115,7 @@
> > > #define AUDIT_REPLACE 1329 /* Replace auditd if this packet unanswerd */
> > > #define AUDIT_KERN_MODULE 1330 /* Kernel Module events */
> > > #define AUDIT_FANOTIFY 1331 /* Fanotify access decision */
> > > +#define AUDIT_CONTAINER 1332 /* Container ID */
> >
> > I'm not sure I'm completely sold on the AUDIT_CONTAINER_ID and
> > AUDIT_CONTAINER record type names. From what I can tell
> > AUDIT_CONTAINER_ID seems to be used for audit container ID management
> > operations, e.g. setting the ID, whereas the AUDIT_CONTAINER is used
> > to tag events with the corresponding audit container ID. Assuming
> > that is correct, it seems like AUDIT_CONTAINER might be better served
> > if it was named AUDIT_CONTAINER_ID and if we could change
> > AUDIT_CONTAINER_ID to AUDIT_CONTAINER_OP/MGMT/etc. Thoughts?
>
> Please see discussion at:
> https://www.redhat.com/archives/linux-audit/2018-May/msg00101.html
>
> I'm fine with changing AUDIT_CONTAINER_ID to AUDIT_CONTAINER_OP/MGMT/etc.
Noted, and while I'm generally a big fan of consistency for things
like this, I think these things are different enough (the loginuid is
recorded as a field, the audit container ID is recorded in a dedicated
record) that we don't need to be bound by LOGINUID's naming
convention.
> > > #define AUDIT_AVC 1400 /* SE Linux avc denial or grant */
> > > #define AUDIT_SELINUX_ERR 1401 /* Internal SE Linux Errors */
> > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > index e7478cb..5e150c6 100644
> > > --- a/kernel/audit.c
> > > +++ b/kernel/audit.c
> > > @@ -2048,6 +2048,29 @@ void audit_log_session_info(struct audit_buffer *ab)
> > > audit_log_format(ab, " auid=%u ses=%u", auid, sessionid);
> > > }
> > >
> > > +/*
> > > + * audit_log_contid - report container info
> > > + * @tsk: task to be recorded
> > > + * @context: task or local context for record
> > > + * @op: contid string description
> > > + */
> > > +int audit_log_contid(struct task_struct *tsk,
> > > + struct audit_context *context, char *op)
> > > +{
> > > + struct audit_buffer *ab;
> > > +
> > > + if (!audit_contid_set(tsk))
> > > + return 0;
> > > + /* Generate AUDIT_CONTAINER record with container ID */
> > > + ab = audit_log_start(context, GFP_KERNEL, AUDIT_CONTAINER);
> > > + if (!ab)
> > > + return -ENOMEM;
> > > + audit_log_format(ab, "op=%s contid=%llu",
> > > + op, audit_get_contid(tsk));
> >
> > Can you explain your reason for including an "op" field in this record
> > type? I've been looking at the rest of the patches in this patchset
> > and it seems to be used more as an indicator of the record's
> > generating context rather than any sort of audit container ID
> > operation.
>
> "action" might work, but that's netfilter and numeric... "kind"?
> Nothing else really seems to fit from a field name, type or lack of
> searchability perspective.
My concern isn't so much the name of the "op" field, although that
does seem wrong, but rather the existence of the field in the first
place. This audit container ID record (whatever we end up calling it)
exists to attach an audit container ID to an audit event, that's it;
an audit event should have other records which provide the context
(granted, the exact number of records depends on the event and the
system's configuration). If we are relying on this record to provide
critical information about the audit event other than the audit
container ID, I believe this is a strong indicator that the existing
audit records are lacking and should be augmented.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* [PATCH] iputils: upgrade to s20190629
From: Ross Burton @ 2018-07-23 13:16 UTC (permalink / raw)
To: openembedded-core
Drop upstreamed patch.
Change license checksums to check the License files themselves (BSD3 and GPL2),
and the tool-to-license file (LICENSE). Changes to LICENSE will now be
detected.
Update libidn build-dependency to libidn2.
Signed-off-by: Ross Burton <ross.burton@intel.com>
---
.../iputils/files/0001-Fix-build-on-MUSL.patch | 95 ----------------------
.../{iputils_s20161105.bb => iputils_s20180629.bb} | 18 ++--
2 files changed, 8 insertions(+), 105 deletions(-)
delete mode 100644 meta/recipes-extended/iputils/files/0001-Fix-build-on-MUSL.patch
rename meta/recipes-extended/iputils/{iputils_s20161105.bb => iputils_s20180629.bb} (70%)
diff --git a/meta/recipes-extended/iputils/files/0001-Fix-build-on-MUSL.patch b/meta/recipes-extended/iputils/files/0001-Fix-build-on-MUSL.patch
deleted file mode 100644
index ce8cfebbb03..00000000000
--- a/meta/recipes-extended/iputils/files/0001-Fix-build-on-MUSL.patch
+++ /dev/null
@@ -1,95 +0,0 @@
-From c6c5966d9fae27bc2f0a34ab59b999555111bd2d Mon Sep 17 00:00:00 2001
-From: Aric Belsito <lluixhi@gmail.com>
-Date: Sun, 29 Oct 2017 23:17:03 +0100
-Subject: [PATCH] Fix build on MUSL
-
-Add missing AI_IDN and NI_IDN declarations.
-
-Bug: https://bugs.gentoo.org/503914
-
-Upstream-Status: Pending
-Signed-off-by: Alexander Kanavin <alex.kanavin@gmail.com>
----
- arping.c | 7 +++++++
- ping.h | 11 +++++++++++
- tracepath.c | 8 ++++++++
- traceroute6.c | 4 ++++
- 4 files changed, 30 insertions(+)
-
-diff --git a/arping.c b/arping.c
-index 1a3f40b..531d29d 100644
---- a/arping.c
-+++ b/arping.c
-@@ -45,6 +45,13 @@ struct sysfs_devattr_values;
- #ifdef USE_IDN
- #include <idna.h>
- #include <locale.h>
-+
-+#ifndef AI_IDN
-+#define AI_IDN 0x0040
-+#endif
-+#ifndef AI_CANONIDN
-+#define AI_CANONIDN 0x0080
-+#endif
- #endif
-
- #include "SNAPSHOT.h"
-diff --git a/ping.h b/ping.h
-index 749f3ff..227315f 100644
---- a/ping.h
-+++ b/ping.h
-@@ -38,6 +38,17 @@
- #include <locale.h>
- #include <idna.h>
- #include <stringprep.h>
-+
-+#ifndef AI_IDN
-+#define AI_IDN 0x0040
-+#endif
-+#ifndef AI_CANONIDN
-+#define AI_CANONIDN 0x0080
-+#endif
-+#ifndef NI_IDN
-+#define NI_IDN 32
-+#endif
-+
- #define getaddrinfo_flags (AI_CANONNAME | AI_IDN | AI_CANONIDN)
- #define getnameinfo_flags NI_IDN
- #else
-diff --git a/tracepath.c b/tracepath.c
-index 74a829d..dcec5b9 100644
---- a/tracepath.c
-+++ b/tracepath.c
-@@ -30,6 +30,14 @@
- #ifdef USE_IDN
- #include <idna.h>
- #include <locale.h>
-+
-+#ifndef AI_IDN
-+#define AI_IDN 0x0040
-+#endif
-+#ifndef NI_IDN
-+#define NI_IDN 32
-+#endif
-+
- #define getnameinfo_flags NI_IDN
- #else
- #define getnameinfo_flags 0
-diff --git a/traceroute6.c b/traceroute6.c
-index 48fc5c5..8d1eebf 100644
---- a/traceroute6.c
-+++ b/traceroute6.c
-@@ -251,6 +251,10 @@
- #include <idna.h>
- #include <locale.h>
-
-+#ifndef NI_IDN
-+#define NI_IDN 32
-+#endif
-+
- #define getnameinfo_flags NI_IDN
- #else
- #define getnameinfo_flags 0
---
-2.15.1
-
diff --git a/meta/recipes-extended/iputils/iputils_s20161105.bb b/meta/recipes-extended/iputils/iputils_s20180629.bb
similarity index 70%
rename from meta/recipes-extended/iputils/iputils_s20161105.bb
rename to meta/recipes-extended/iputils/iputils_s20180629.bb
index 0125739b034..d421cc34e77 100644
--- a/meta/recipes-extended/iputils/iputils_s20161105.bb
+++ b/meta/recipes-extended/iputils/iputils_s20180629.bb
@@ -6,25 +6,23 @@ SECTION = "console/network"
LICENSE = "BSD & GPLv2+"
-LIC_FILES_CHKSUM = "file://ping.c;beginline=1;endline=35;md5=f9ceb201733e9a6cf8f00766dd278d82 \
- file://tracepath.c;beginline=1;endline=10;md5=0ecea2bf60bff2f3d840096d87647f3d \
- file://arping.c;beginline=1;endline=11;md5=fe84301b5c2655c950f8b92a057fafa6 \
- file://tftpd.c;beginline=1;endline=32;md5=28834bf8a91a5b8a92755dbee709ef96 "
+LIC_FILES_CHKSUM = "file://LICENSE;md5=b792e38abdc59f766a3153908f23e766 \
+ file://LICENSE.BSD3;md5=0f00d99239d922ffd13cabef83b33444 \
+ file://LICENSE.GPL2;md5=b234ee4d69f5fce4486a80fdaf4a4263"
DEPENDS = "gnutls libcap libgcrypt"
-SRC_URI = "git://github.com/iputils/iputils \
- file://0001-Fix-build-on-MUSL.patch \
- "
+SRC_URI = "git://github.com/iputils/iputils"
+SRCREV = "f6aac8dbe3f8c45c53424854a3312bdd8cdd58d3"
+
S = "${WORKDIR}/git"
-SRCREV = "bffc0e957b98d626ab4cea218c89251201425442"
UPSTREAM_CHECK_GITTAGREGEX = "(?P<pver>s\d+)"
EXTRA_OEMAKE = "-e MAKEFLAGS="
-PACKAGECONFIG ?= ""
-PACKAGECONFIG[libidn] = "USE_IDN=yes,USE_IDN=no,libidn"
+PACKAGECONFIG ??= ""
+PACKAGECONFIG[libidn] = "USE_IDN=yes,USE_IDN=no,libidn2"
do_compile () {
oe_runmake 'CC=${CC} -D_GNU_SOURCE' VPATH="${STAGING_LIBDIR}:${STAGING_DIR_HOST}/${base_libdir}" ${PACKAGECONFIG_CONFARGS} all
--
2.11.0
^ permalink raw reply related
* Re: potential deadlock in cpufreq-dt
From: Eugeniu Rosca @ 2018-07-23 13:15 UTC (permalink / raw)
To: jiada, Viresh Kumar; +Cc: rjw, linux-kernel, linux-pm, Eugeniu Rosca
In-Reply-To: <3098ae70-85b6-9c37-c177-63652eeda804@mentor.com>
Hi all,
[since Jiada's initial report was based on 4.14.50+]
FWIW, I can consistently reproduce this lockdep issue on 4.18.0-rc6:
[ 17.119559] ======================================================
[ 17.119571] WARNING: possible circular locking dependency detected
[ 17.119589] 4.18.0-rc6 #3 Not tainted
[ 17.119601] ------------------------------------------------------
[ 17.119617] systemd-udevd/1538 is trying to acquire lock:
[ 17.119631] (____ptrval____) (&sb->s_type->i_mutex_key
[ 17.137496] #3){+.+.}, at: start_creating+0xbc/0x140
[ 17.137573]
[ 17.137573] but task is already holding lock:
[ 17.137597] (____ptrval____) (opp_table_lock){+.+.}, at: dev_pm_opp_get_opp_table+0xac/0x310
[ 17.181185]
[ 17.181185] which lock already depends on the new lock.
[ 17.189506]
[ 17.189506] the existing dependency chain (in reverse order) is:
[ 17.197116]
[ 17.197116] -> #3 (opp_table_lock){+.+.}:
[ 17.202803] lock_acquire+0x578/0x5c0
[ 17.207101] __mutex_lock+0xec/0xdf8
[ 17.211309] mutex_lock_nested+0x3c/0x4c
[ 17.215866] dev_pm_opp_get_opp_table+0xac/0x310
[ 17.221121] dev_pm_opp_set_regulators+0x38/0x300
[ 17.226506] cpufreq_init+0x198/0x3e0 [cpufreq_dt]
[ 17.231950] cpufreq_online+0x3f0/0xcd4
[ 17.236425] cpufreq_add_dev+0x164/0x17c
[ 17.240988] subsys_interface_register+0x244/0x264
[ 17.246409] cpufreq_register_driver+0x178/0x278
[ 17.251680] dt_cpufreq_probe+0x158/0x178 [cpufreq_dt]
[ 17.257435] platform_drv_probe+0xa4/0x134
[ 17.262167] driver_probe_device+0x36c/0x488
[ 17.267085] __driver_attach+0x150/0x1e4
[ 17.271642] bus_for_each_dev+0x110/0x138
[ 17.271677] driver_attach+0x50/0x5c
[ 17.281065] bus_add_driver+0x2ac/0x2f8
[ 17.281093] driver_register+0x180/0x1c8
[ 17.281124] __platform_driver_register+0x9c/0xac
[ 17.289056] dt_cpufreq_platdrv_init+0x28/0x1000 [cpufreq_dt]
[ 17.289093] do_one_initcall+0x59c/0xab0
[ 17.298045] do_init_module+0x174/0x450
[ 17.298073] load_module+0x3a40/0x3b60
[ 17.298101] sys_finit_module+0x150/0x168
[ 17.309161] el0_svc_naked+0x30/0x34
[ 17.309176]
[ 17.309176] -> #2 (
[ 17.325197] subsys mutex#6){+.+.}:
[ 17.334020] lock_acquire+0x578/0x5c0
[ 17.334048] __mutex_lock+0xec/0xdf8
[ 17.334069] mutex_lock_nested+0x3c/0x4c
[ 17.375822] subsys_interface_register+0x10c/0x264
[ 17.381268] cpufreq_register_driver+0x178/0x278
[ 17.386563] dt_cpufreq_probe+0x158/0x178 [cpufreq_dt]
[ 17.392355] platform_drv_probe+0xa4/0x134
[ 17.397089] driver_probe_device+0x36c/0x488
[ 17.402001] __driver_attach+0x150/0x1e4
[ 17.406556] bus_for_each_dev+0x110/0x138
[ 17.411202] driver_attach+0x50/0x5c
[ 17.415409] bus_add_driver+0x2ac/0x2f8
[ 17.419881] driver_register+0x180/0x1c8
[ 17.424442] __platform_driver_register+0x9c/0xac
[ 17.429825] dt_cpufreq_platdrv_init+0x28/0x1000 [cpufreq_dt]
[ 17.436234] do_one_initcall+0x59c/0xab0
[ 17.440799] do_init_module+0x174/0x450
[ 17.445243] load_module+0x3a40/0x3b60
[ 17.449588] sys_finit_module+0x150/0x168
[ 17.454193] el0_svc_naked+0x30/0x34
[ 17.458350]
[ 17.458350] -> #1 (cpu_hotplug_lock.rw_sem){++++}:
[ 17.464779] lock_acquire+0x578/0x5c0
[ 17.469033] cpus_read_lock+0x60/0x130
[ 17.473379] static_key_enable+0x1c/0x34
[ 17.477894] sched_feat_write+0x300/0x3ec
[ 17.482501] full_proxy_write+0xa4/0xc8
[ 17.486935] __vfs_write+0x10c/0x368
[ 17.491100] vfs_write+0x144/0x2e8
[ 17.495090] ksys_write+0xcc/0x13c
[ 17.499079] sys_write+0x34/0x44
[ 17.502892] el0_svc_naked+0x30/0x34
[ 17.507046]
[ 17.507046] -> #0 (&sb->s_type->i_mutex_key#3){+.+.}:
[ 17.513742] __lock_acquire+0x1968/0x1f4c
[ 17.518348] lock_acquire+0x578/0x5c0
[ 17.522606] down_write+0xb8/0x148
[ 17.526602] start_creating+0xbc/0x140
[ 17.530948] debugfs_create_dir+0x2c/0x1b0
[ 17.535642] opp_debug_register+0xc4/0x10c
[ 17.540330] _add_opp_dev+0x178/0x1d8
[ 17.544581] dev_pm_opp_get_opp_table+0x134/0x310
[ 17.549885] dev_pm_opp_set_regulators+0x38/0x300
[ 17.555224] cpufreq_init+0x198/0x3e0 [cpufreq_dt]
[ 17.560622] cpufreq_online+0x3f0/0xcd4
[ 17.565052] cpufreq_add_dev+0x164/0x17c
[ 17.569574] subsys_interface_register+0x244/0x264
[ 17.574968] cpufreq_register_driver+0x178/0x278
[ 17.580211] dt_cpufreq_probe+0x158/0x178 [cpufreq_dt]
[ 17.585953] platform_drv_probe+0xa4/0x134
[ 17.590647] driver_probe_device+0x36c/0x488
[ 17.595517] __driver_attach+0x150/0x1e4
[ 17.600034] bus_for_each_dev+0x110/0x138
[ 17.604640] driver_attach+0x50/0x5c
[ 17.608808] bus_add_driver+0x2ac/0x2f8
[ 17.613240] driver_register+0x180/0x1c8
[ 17.617761] __platform_driver_register+0x9c/0xac
[ 17.623092] dt_cpufreq_platdrv_init+0x28/0x1000 [cpufreq_dt]
[ 17.629449] do_one_initcall+0x59c/0xab0
[ 17.633970] do_init_module+0x174/0x450
[ 17.638402] load_module+0x3a40/0x3b60
[ 17.642747] sys_finit_module+0x150/0x168
[ 17.647350] el0_svc_naked+0x30/0x34
[ 17.651506]
[ 17.651506] other info that might help us debug this:
[ 17.651506]
[ 17.659624] Chain exists of:
[ 17.659624] &sb->s_type->i_mutex_key#3 --> subsys mutex#6 --> opp_table_lock
[ 17.659624]
[ 17.671440] Possible unsafe locking scenario:
[ 17.671440]
[ 17.677445] CPU0 CPU1
[ 17.682039] ---- ----
[ 17.686631] lock(opp_table_lock);
[ 17.690197] lock(subsys mutex#6);
[ 17.696315] lock(opp_table_lock);
[ 17.702421] lock(&sb->s_type->i_mutex_key#3);
[ 17.707051]
[ 17.707051] *** DEADLOCK ***
[ 17.707051]
[ 17.713070] 4 locks held by systemd-udevd/1538:
[ 17.717663] #0: (____ptrval____) (&dev->mutex){....}, at: __driver_attach+0x134/0x1e4
[ 17.725760] #1: (____ptrval____) (cpu_hotplug_lock.rw_sem){++++}, at: cpufreq_register_driver+0xd8/0x278
[ 17.735517] #2: (____ptrval____) (subsys mutex#6){+.+.}, at: subsys_interface_register+0x10c/0x264
[ 17.744761] #3: (____ptrval____) (opp_table_lock){+.+.}, at: dev_pm_opp_get_opp_table+0xac/0x310
[ 17.753811]
[ 17.753811] stack backtrace:
[ 17.758252] CPU: 4 PID: 1538 Comm: systemd-udevd Not tainted 4.18.0-rc6 #3
[ 17.765215] Hardware name: Renesas Salvator-X board based on r8a7795 ES2.0+ (DT)
[ 17.772708] Call trace:
[ 17.775209] dump_backtrace+0x0/0x27c
[ 17.778931] show_stack+0x24/0x30
[ 17.782313] dump_stack+0x13c/0x1b0
[ 17.785868] print_circular_bug.isra.12+0x2a8/0x3f8
[ 17.790826] check_prev_add.constprop.20+0x1a4/0xb24
[ 17.795869] __lock_acquire+0x1968/0x1f4c
[ 17.799948] lock_acquire+0x578/0x5c0
[ 17.803675] down_write+0xb8/0x148
[ 17.807143] start_creating+0xbc/0x140
[ 17.810962] debugfs_create_dir+0x2c/0x1b0
[ 17.815125] opp_debug_register+0xc4/0x10c
[ 17.819287] _add_opp_dev+0x178/0x1d8
[ 17.823012] dev_pm_opp_get_opp_table+0x134/0x310
[ 17.827789] dev_pm_opp_set_regulators+0x38/0x300
[ 17.832596] cpufreq_init+0x198/0x3e0 [cpufreq_dt]
[ 17.837464] cpufreq_online+0x3f0/0xcd4
[ 17.841366] cpufreq_add_dev+0x164/0x17c
[ 17.845360] subsys_interface_register+0x244/0x264
[ 17.850228] cpufreq_register_driver+0x178/0x278
[ 17.854943] dt_cpufreq_probe+0x158/0x178 [cpufreq_dt]
[ 17.860158] platform_drv_probe+0xa4/0x134
[ 17.864326] driver_probe_device+0x36c/0x488
[ 17.868670] __driver_attach+0x150/0x1e4
[ 17.872661] bus_for_each_dev+0x110/0x138
[ 17.876740] driver_attach+0x50/0x5c
[ 17.880381] bus_add_driver+0x2ac/0x2f8
[ 17.884287] driver_register+0x180/0x1c8
[ 17.888283] __platform_driver_register+0x9c/0xac
[ 17.893087] dt_cpufreq_platdrv_init+0x28/0x1000 [cpufreq_dt]
[ 17.898918] do_one_initcall+0x59c/0xab0
[ 17.902912] do_init_module+0x174/0x450
[ 17.906818] load_module+0x3a40/0x3b60
[ 17.910636] sys_finit_module+0x150/0x168
[ 17.914712] el0_svc_naked+0x30/0x34
^ permalink raw reply
* Re: [PATCH] nohz: don't kick non-idle CPUs in tick_nohz_full_kick_cpu()
From: Frederic Weisbecker @ 2018-07-23 13:15 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Yury Norov, Frederic Weisbecker, Ingo Molnar, Goutham, Sunil,
Chris Metcalf, linux-kernel
In-Reply-To: <alpine.DEB.2.21.1807201916270.1694@nanos.tec.linutronix.de>
On Fri, Jul 20, 2018 at 07:24:00PM +0200, Thomas Gleixner wrote:
> On Thu, 19 Jul 2018, Yury Norov wrote:
> > While here. I just wonder, on my system IRQs are sent to nohz_full CPUs
> > at every incoming ssh connection. The trace is like this:
> > [ 206.835533] Call trace:
> > [ 206.848411] [<ffff00000889f984>] dump_stack+0x84/0xa8
> > [ 206.853455] [<ffff0000081ea308>] _task_isolation_remote+0x130/0x140
> > [ 206.859714] [<ffff0000081bf5ec>] irq_work_queue_on+0xcc/0xfc
> > [ 206.865365] [<ffff0000081478ac>] tick_nohz_full_kick_cpu+0x88/0x94
> > [ 206.871536] [<ffff000008147930>] tick_nohz_dep_set_all+0x78/0xa8
> > [ 206.877533] [<ffff000008147b58>] tick_nohz_dep_set_signal+0x28/0x34
> > [ 206.883792] [<ffff0000081421fc>] set_process_cpu_timer+0xd0/0x128
> > [ 206.889876] [<ffff0000081422ac>] update_rlimit_cpu+0x58/0x7c
> > [ 206.895528] [<ffff0000083aa3d0>] selinux_bprm_committing_creds+0x180/0x1fc
> > [ 206.902394] [<ffff00000839e394>] security_bprm_committing_creds+0x40/0x5c
> > [ 206.909173] [<ffff00000828c4a0>] install_exec_creds+0x20/0x6c
> > [ 206.914911] [<ffff0000082e15b0>] load_elf_binary+0x368/0xbb8
> > [ 206.920561] [<ffff00000828d09c>] search_binary_handler+0xb8/0x224
> > [ 206.926645] [<ffff00000828d99c>] do_execveat_common+0x44c/0x5f0
> > [ 206.932555] [<ffff00000828db78>] do_execve+0x38/0x44
> > [ 206.937510] [<ffff00000828dd74>] SyS_execve+0x34/0x44
> >
> > I suspect that scp, ssh tunneling and similar network activities will source
> > ticks on nohz_full CPUs as well. On high-loaded server it may generate
> > significant interrupt traffic on nohz_full CPUs. Is it desirable behavior?
>
> Supsicions and desirable are not really technical interesting aspects.
>
> Just from looking at the stack trace it's obvious that there is a CPU TIME
> rlimit on that newly spawned sshd. That's nothing what the kernel
> imposes. That's what user space sets.
>
> Now the actual mechanism which does that, i.e. set_process_cpu_timer() ends
> up IPI'ing _ALL_ nohz full CPUs for no real good reason. In the exec path
> this is really pointless because the new process is not running yet and it
> is single threaded. So forcing a IPI to all cpus is pretty pointless.
>
> In fact the state of the task/process for which update_rlimit_cpu(() is
> called is known, so the IPI can really be either avoided completely or
> restricted to the CPUs on which this process can run or actually runs.
>
> Fredric?
Indeed, so far the tick dependency code is lazy and IPIs everywhere when we
add either a thread or a process timer.
We want to make sure that any thread target, running somewhere without a tick,
sees the new tick dependency.
So in the case a single thread, I can easily fix that and IPI the CPU it's running
in if any. In the case of a thread group, I'm concerned about the performance
penalty to walk through each of them and IPI only those running. But probably we'll
have to come into that in the end.
Thanks.
^ permalink raw reply
* Re: potential deadlock in cpufreq-dt
From: Eugeniu Rosca @ 2018-07-23 13:15 UTC (permalink / raw)
To: jiada, Viresh Kumar; +Cc: rjw, linux-kernel, linux-pm, Eugeniu Rosca
In-Reply-To: <3098ae70-85b6-9c37-c177-63652eeda804@mentor.com>
Hi all,
[since Jiada's initial report was based on 4.14.50+]
FWIW, I can consistently reproduce this lockdep issue on 4.18.0-rc6:
[ 17.119559] ======================================================
[ 17.119571] WARNING: possible circular locking dependency detected
[ 17.119589] 4.18.0-rc6 #3 Not tainted
[ 17.119601] ------------------------------------------------------
[ 17.119617] systemd-udevd/1538 is trying to acquire lock:
[ 17.119631] (____ptrval____) (&sb->s_type->i_mutex_key
[ 17.137496] #3){+.+.}, at: start_creating+0xbc/0x140
[ 17.137573]
[ 17.137573] but task is already holding lock:
[ 17.137597] (____ptrval____) (opp_table_lock){+.+.}, at: dev_pm_opp_get_opp_table+0xac/0x310
[ 17.181185]
[ 17.181185] which lock already depends on the new lock.
[ 17.189506]
[ 17.189506] the existing dependency chain (in reverse order) is:
[ 17.197116]
[ 17.197116] -> #3 (opp_table_lock){+.+.}:
[ 17.202803] lock_acquire+0x578/0x5c0
[ 17.207101] __mutex_lock+0xec/0xdf8
[ 17.211309] mutex_lock_nested+0x3c/0x4c
[ 17.215866] dev_pm_opp_get_opp_table+0xac/0x310
[ 17.221121] dev_pm_opp_set_regulators+0x38/0x300
[ 17.226506] cpufreq_init+0x198/0x3e0 [cpufreq_dt]
[ 17.231950] cpufreq_online+0x3f0/0xcd4
[ 17.236425] cpufreq_add_dev+0x164/0x17c
[ 17.240988] subsys_interface_register+0x244/0x264
[ 17.246409] cpufreq_register_driver+0x178/0x278
[ 17.251680] dt_cpufreq_probe+0x158/0x178 [cpufreq_dt]
[ 17.257435] platform_drv_probe+0xa4/0x134
[ 17.262167] driver_probe_device+0x36c/0x488
[ 17.267085] __driver_attach+0x150/0x1e4
[ 17.271642] bus_for_each_dev+0x110/0x138
[ 17.271677] driver_attach+0x50/0x5c
[ 17.281065] bus_add_driver+0x2ac/0x2f8
[ 17.281093] driver_register+0x180/0x1c8
[ 17.281124] __platform_driver_register+0x9c/0xac
[ 17.289056] dt_cpufreq_platdrv_init+0x28/0x1000 [cpufreq_dt]
[ 17.289093] do_one_initcall+0x59c/0xab0
[ 17.298045] do_init_module+0x174/0x450
[ 17.298073] load_module+0x3a40/0x3b60
[ 17.298101] sys_finit_module+0x150/0x168
[ 17.309161] el0_svc_naked+0x30/0x34
[ 17.309176]
[ 17.309176] -> #2 (
[ 17.325197] subsys mutex#6){+.+.}:
[ 17.334020] lock_acquire+0x578/0x5c0
[ 17.334048] __mutex_lock+0xec/0xdf8
[ 17.334069] mutex_lock_nested+0x3c/0x4c
[ 17.375822] subsys_interface_register+0x10c/0x264
[ 17.381268] cpufreq_register_driver+0x178/0x278
[ 17.386563] dt_cpufreq_probe+0x158/0x178 [cpufreq_dt]
[ 17.392355] platform_drv_probe+0xa4/0x134
[ 17.397089] driver_probe_device+0x36c/0x488
[ 17.402001] __driver_attach+0x150/0x1e4
[ 17.406556] bus_for_each_dev+0x110/0x138
[ 17.411202] driver_attach+0x50/0x5c
[ 17.415409] bus_add_driver+0x2ac/0x2f8
[ 17.419881] driver_register+0x180/0x1c8
[ 17.424442] __platform_driver_register+0x9c/0xac
[ 17.429825] dt_cpufreq_platdrv_init+0x28/0x1000 [cpufreq_dt]
[ 17.436234] do_one_initcall+0x59c/0xab0
[ 17.440799] do_init_module+0x174/0x450
[ 17.445243] load_module+0x3a40/0x3b60
[ 17.449588] sys_finit_module+0x150/0x168
[ 17.454193] el0_svc_naked+0x30/0x34
[ 17.458350]
[ 17.458350] -> #1 (cpu_hotplug_lock.rw_sem){++++}:
[ 17.464779] lock_acquire+0x578/0x5c0
[ 17.469033] cpus_read_lock+0x60/0x130
[ 17.473379] static_key_enable+0x1c/0x34
[ 17.477894] sched_feat_write+0x300/0x3ec
[ 17.482501] full_proxy_write+0xa4/0xc8
[ 17.486935] __vfs_write+0x10c/0x368
[ 17.491100] vfs_write+0x144/0x2e8
[ 17.495090] ksys_write+0xcc/0x13c
[ 17.499079] sys_write+0x34/0x44
[ 17.502892] el0_svc_naked+0x30/0x34
[ 17.507046]
[ 17.507046] -> #0 (&sb->s_type->i_mutex_key#3){+.+.}:
[ 17.513742] __lock_acquire+0x1968/0x1f4c
[ 17.518348] lock_acquire+0x578/0x5c0
[ 17.522606] down_write+0xb8/0x148
[ 17.526602] start_creating+0xbc/0x140
[ 17.530948] debugfs_create_dir+0x2c/0x1b0
[ 17.535642] opp_debug_register+0xc4/0x10c
[ 17.540330] _add_opp_dev+0x178/0x1d8
[ 17.544581] dev_pm_opp_get_opp_table+0x134/0x310
[ 17.549885] dev_pm_opp_set_regulators+0x38/0x300
[ 17.555224] cpufreq_init+0x198/0x3e0 [cpufreq_dt]
[ 17.560622] cpufreq_online+0x3f0/0xcd4
[ 17.565052] cpufreq_add_dev+0x164/0x17c
[ 17.569574] subsys_interface_register+0x244/0x264
[ 17.574968] cpufreq_register_driver+0x178/0x278
[ 17.580211] dt_cpufreq_probe+0x158/0x178 [cpufreq_dt]
[ 17.585953] platform_drv_probe+0xa4/0x134
[ 17.590647] driver_probe_device+0x36c/0x488
[ 17.595517] __driver_attach+0x150/0x1e4
[ 17.600034] bus_for_each_dev+0x110/0x138
[ 17.604640] driver_attach+0x50/0x5c
[ 17.608808] bus_add_driver+0x2ac/0x2f8
[ 17.613240] driver_register+0x180/0x1c8
[ 17.617761] __platform_driver_register+0x9c/0xac
[ 17.623092] dt_cpufreq_platdrv_init+0x28/0x1000 [cpufreq_dt]
[ 17.629449] do_one_initcall+0x59c/0xab0
[ 17.633970] do_init_module+0x174/0x450
[ 17.638402] load_module+0x3a40/0x3b60
[ 17.642747] sys_finit_module+0x150/0x168
[ 17.647350] el0_svc_naked+0x30/0x34
[ 17.651506]
[ 17.651506] other info that might help us debug this:
[ 17.651506]
[ 17.659624] Chain exists of:
[ 17.659624] &sb->s_type->i_mutex_key#3 --> subsys mutex#6 --> opp_table_lock
[ 17.659624]
[ 17.671440] Possible unsafe locking scenario:
[ 17.671440]
[ 17.677445] CPU0 CPU1
[ 17.682039] ---- ----
[ 17.686631] lock(opp_table_lock);
[ 17.690197] lock(subsys mutex#6);
[ 17.696315] lock(opp_table_lock);
[ 17.702421] lock(&sb->s_type->i_mutex_key#3);
[ 17.707051]
[ 17.707051] *** DEADLOCK ***
[ 17.707051]
[ 17.713070] 4 locks held by systemd-udevd/1538:
[ 17.717663] #0: (____ptrval____) (&dev->mutex){....}, at: __driver_attach+0x134/0x1e4
[ 17.725760] #1: (____ptrval____) (cpu_hotplug_lock.rw_sem){++++}, at: cpufreq_register_driver+0xd8/0x278
[ 17.735517] #2: (____ptrval____) (subsys mutex#6){+.+.}, at: subsys_interface_register+0x10c/0x264
[ 17.744761] #3: (____ptrval____) (opp_table_lock){+.+.}, at: dev_pm_opp_get_opp_table+0xac/0x310
[ 17.753811]
[ 17.753811] stack backtrace:
[ 17.758252] CPU: 4 PID: 1538 Comm: systemd-udevd Not tainted 4.18.0-rc6 #3
[ 17.765215] Hardware name: Renesas Salvator-X board based on r8a7795 ES2.0+ (DT)
[ 17.772708] Call trace:
[ 17.775209] dump_backtrace+0x0/0x27c
[ 17.778931] show_stack+0x24/0x30
[ 17.782313] dump_stack+0x13c/0x1b0
[ 17.785868] print_circular_bug.isra.12+0x2a8/0x3f8
[ 17.790826] check_prev_add.constprop.20+0x1a4/0xb24
[ 17.795869] __lock_acquire+0x1968/0x1f4c
[ 17.799948] lock_acquire+0x578/0x5c0
[ 17.803675] down_write+0xb8/0x148
[ 17.807143] start_creating+0xbc/0x140
[ 17.810962] debugfs_create_dir+0x2c/0x1b0
[ 17.815125] opp_debug_register+0xc4/0x10c
[ 17.819287] _add_opp_dev+0x178/0x1d8
[ 17.823012] dev_pm_opp_get_opp_table+0x134/0x310
[ 17.827789] dev_pm_opp_set_regulators+0x38/0x300
[ 17.832596] cpufreq_init+0x198/0x3e0 [cpufreq_dt]
[ 17.837464] cpufreq_online+0x3f0/0xcd4
[ 17.841366] cpufreq_add_dev+0x164/0x17c
[ 17.845360] subsys_interface_register+0x244/0x264
[ 17.850228] cpufreq_register_driver+0x178/0x278
[ 17.854943] dt_cpufreq_probe+0x158/0x178 [cpufreq_dt]
[ 17.860158] platform_drv_probe+0xa4/0x134
[ 17.864326] driver_probe_device+0x36c/0x488
[ 17.868670] __driver_attach+0x150/0x1e4
[ 17.872661] bus_for_each_dev+0x110/0x138
[ 17.876740] driver_attach+0x50/0x5c
[ 17.880381] bus_add_driver+0x2ac/0x2f8
[ 17.884287] driver_register+0x180/0x1c8
[ 17.888283] __platform_driver_register+0x9c/0xac
[ 17.893087] dt_cpufreq_platdrv_init+0x28/0x1000 [cpufreq_dt]
[ 17.898918] do_one_initcall+0x59c/0xab0
[ 17.902912] do_init_module+0x174/0x450
[ 17.906818] load_module+0x3a40/0x3b60
[ 17.910636] sys_finit_module+0x150/0x168
[ 17.914712] el0_svc_naked+0x30/0x34
^ permalink raw reply
* Re: request for 4.4-stable: 1dc3039bc87a ("block: do not use interruptible wait anywhere")
From: Greg Kroah-Hartman @ 2018-07-23 12:14 UTC (permalink / raw)
To: Sudip Mukherjee; +Cc: stable
In-Reply-To: <20180723092038.22eg3o6zszojgvbn@debian>
On Mon, Jul 23, 2018 at 10:20:38AM +0100, Sudip Mukherjee wrote:
> Hi Greg,
>
> As discussed with Alan, this is the backported patch for 4.4-stable.
> Please apply it to your queue.
Now applied, thanks.
greg k-h
^ permalink raw reply
* Re: [PATCH] bcache: set max writeback rate when I/O request is idle
From: Kai Krakow @ 2018-07-23 13:14 UTC (permalink / raw)
To: Coly Li; +Cc: linux-bcache, linux-block, stable, Michael Lyle
In-Reply-To: <20180722161316.2347-1-colyli@suse.de>
2018-07-22 18:13 GMT+02:00 Coly Li <colyli@suse.de>:
> Commit b1092c9af9ed ("bcache: allow quick writeback when backing idle")
> allows the writeback rate to be faster if there is no I/O request on a
> bcache device. It works well if there is only one bcache device attached
> to the cache set. If there are many bcache devices attached to a cache
> set, it may introduce performance regression because multiple faster
> writeback threads of the idle bcache devices will compete the btree level
> locks with the bcache device who have I/O requests coming.
>
> This patch fixes the above issue by only permitting fast writebac when
> all bcache devices attached on the cache set are idle. And if one of the
> bcache devices has new I/O request coming, minimized all writeback
> throughput immediately and let PI controller __update_writeback_rate()
> to decide the upcoming writeback rate for each bcache device.
>
> Also when all bcache devices are idle, limited wrieback rate to a small
> number is wast of thoughput, especially when backing devices are slower
> non-rotation devices (e.g. SATA SSD). This patch sets a max writeback
> rate for each backing device if the whole cache set is idle. A faster
> writeback rate in idle time means new I/Os may have more available space
> for dirty data, and people may observe a better write performance then.
>
> Please note bcache may change its cache mode in run time, and this patch
> still works if the cache mode is switched from writeback mode and there
> is still dirty data on cache.
Running with this patch on 4.17, I see reduced IO lags in desktop
usage during heavy writes. The reduction is minor but noticeable. The
small short IO lags seem to be fixed with this but I still see the
stalls with long lags in desktop usage. I don't address those to
bcache, tho. This seems more like an issue resulting from using bees
on btrfs.
> Fixes: Commit b1092c9af9ed ("bcache: allow quick writeback when backing idle")
> Cc: stable@vger.kernel.org #4.16+
> Signed-off-by: Coly Li <colyli@suse.de>
> Cc: Michael Lyle <mlyle@lyle.org>
Tested-by: Kai Krakow <kai@kaishome.de>
> ---
> drivers/md/bcache/bcache.h | 9 +---
> drivers/md/bcache/request.c | 42 ++++++++++++++-
> drivers/md/bcache/sysfs.c | 14 +++--
> drivers/md/bcache/util.c | 2 +-
> drivers/md/bcache/util.h | 2 +-
> drivers/md/bcache/writeback.c | 98 +++++++++++++++++++++++++----------
> 6 files changed, 126 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h
> index d6bf294f3907..f7451e8be03c 100644
> --- a/drivers/md/bcache/bcache.h
> +++ b/drivers/md/bcache/bcache.h
> @@ -328,13 +328,6 @@ struct cached_dev {
> */
> atomic_t has_dirty;
>
> - /*
> - * Set to zero by things that touch the backing volume-- except
> - * writeback. Incremented by writeback. Used to determine when to
> - * accelerate idle writeback.
> - */
> - atomic_t backing_idle;
> -
> struct bch_ratelimit writeback_rate;
> struct delayed_work writeback_rate_update;
>
> @@ -514,6 +507,8 @@ struct cache_set {
> struct cache_accounting accounting;
>
> unsigned long flags;
> + atomic_t idle_counter;
> + atomic_t at_max_writeback_rate;
>
> struct cache_sb sb;
>
> diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
> index ae67f5fa8047..fe45f561a054 100644
> --- a/drivers/md/bcache/request.c
> +++ b/drivers/md/bcache/request.c
> @@ -1104,6 +1104,34 @@ static void detached_dev_do_request(struct bcache_device *d, struct bio *bio)
>
> /* Cached devices - read & write stuff */
>
> +static void quit_max_writeback_rate(struct cache_set *c)
> +{
> + int i;
> + struct bcache_device *d;
> + struct cached_dev *dc;
> +
> + mutex_lock(&bch_register_lock);
> +
> + for (i = 0; i < c->devices_max_used; i++) {
> + if (!c->devices[i])
> + continue;
> +
> + if (UUID_FLASH_ONLY(&c->uuids[i]))
> + continue;
> +
> + d = c->devices[i];
> + dc = container_of(d, struct cached_dev, disk);
> + /*
> + * set writeback rate to default minimum value,
> + * then let update_writeback_rate() to decide the
> + * upcoming rate.
> + */
> + atomic64_set(&dc->writeback_rate.rate, 1);
> + }
> +
> + mutex_unlock(&bch_register_lock);
> +}
> +
> static blk_qc_t cached_dev_make_request(struct request_queue *q,
> struct bio *bio)
> {
> @@ -1119,7 +1147,19 @@ static blk_qc_t cached_dev_make_request(struct request_queue *q,
> return BLK_QC_T_NONE;
> }
>
> - atomic_set(&dc->backing_idle, 0);
> + if (d->c) {
> + atomic_set(&d->c->idle_counter, 0);
> + /*
> + * If at_max_writeback_rate of cache set is true and new I/O
> + * comes, quit max writeback rate of all cached devices
> + * attached to this cache set, and set at_max_writeback_rate
> + * to false.
> + */
> + if (unlikely(atomic_read(&d->c->at_max_writeback_rate) == 1)) {
> + atomic_set(&d->c->at_max_writeback_rate, 0);
> + quit_max_writeback_rate(d->c);
> + }
> + }
> generic_start_io_acct(q, rw, bio_sectors(bio), &d->disk->part0);
>
> bio_set_dev(bio, dc->bdev);
> diff --git a/drivers/md/bcache/sysfs.c b/drivers/md/bcache/sysfs.c
> index 225b15aa0340..d719021bff81 100644
> --- a/drivers/md/bcache/sysfs.c
> +++ b/drivers/md/bcache/sysfs.c
> @@ -170,7 +170,8 @@ SHOW(__bch_cached_dev)
> var_printf(writeback_running, "%i");
> var_print(writeback_delay);
> var_print(writeback_percent);
> - sysfs_hprint(writeback_rate, dc->writeback_rate.rate << 9);
> + sysfs_hprint(writeback_rate,
> + atomic64_read(&dc->writeback_rate.rate) << 9);
> sysfs_hprint(io_errors, atomic_read(&dc->io_errors));
> sysfs_printf(io_error_limit, "%i", dc->error_limit);
> sysfs_printf(io_disable, "%i", dc->io_disable);
> @@ -188,7 +189,8 @@ SHOW(__bch_cached_dev)
> char change[20];
> s64 next_io;
>
> - bch_hprint(rate, dc->writeback_rate.rate << 9);
> + bch_hprint(rate,
> + atomic64_read(&dc->writeback_rate.rate) << 9);
> bch_hprint(dirty, bcache_dev_sectors_dirty(&dc->disk) << 9);
> bch_hprint(target, dc->writeback_rate_target << 9);
> bch_hprint(proportional,dc->writeback_rate_proportional << 9);
> @@ -255,8 +257,12 @@ STORE(__cached_dev)
>
> sysfs_strtoul_clamp(writeback_percent, dc->writeback_percent, 0, 40);
>
> - sysfs_strtoul_clamp(writeback_rate,
> - dc->writeback_rate.rate, 1, INT_MAX);
> + if (attr == &sysfs_writeback_rate) {
> + int v;
> +
> + sysfs_strtoul_clamp(writeback_rate, v, 1, INT_MAX);
> + atomic64_set(&dc->writeback_rate.rate, v);
> + }
>
> sysfs_strtoul_clamp(writeback_rate_update_seconds,
> dc->writeback_rate_update_seconds,
> diff --git a/drivers/md/bcache/util.c b/drivers/md/bcache/util.c
> index fc479b026d6d..84f90c3d996d 100644
> --- a/drivers/md/bcache/util.c
> +++ b/drivers/md/bcache/util.c
> @@ -200,7 +200,7 @@ uint64_t bch_next_delay(struct bch_ratelimit *d, uint64_t done)
> {
> uint64_t now = local_clock();
>
> - d->next += div_u64(done * NSEC_PER_SEC, d->rate);
> + d->next += div_u64(done * NSEC_PER_SEC, atomic64_read(&d->rate));
>
> /* Bound the time. Don't let us fall further than 2 seconds behind
> * (this prevents unnecessary backlog that would make it impossible
> diff --git a/drivers/md/bcache/util.h b/drivers/md/bcache/util.h
> index cced87f8eb27..7e17f32ab563 100644
> --- a/drivers/md/bcache/util.h
> +++ b/drivers/md/bcache/util.h
> @@ -442,7 +442,7 @@ struct bch_ratelimit {
> * Rate at which we want to do work, in units per second
> * The units here correspond to the units passed to bch_next_delay()
> */
> - uint32_t rate;
> + atomic64_t rate;
> };
>
> static inline void bch_ratelimit_reset(struct bch_ratelimit *d)
> diff --git a/drivers/md/bcache/writeback.c b/drivers/md/bcache/writeback.c
> index ad45ebe1a74b..72059f910230 100644
> --- a/drivers/md/bcache/writeback.c
> +++ b/drivers/md/bcache/writeback.c
> @@ -49,6 +49,63 @@ static uint64_t __calc_target_rate(struct cached_dev *dc)
> return (cache_dirty_target * bdev_share) >> WRITEBACK_SHARE_SHIFT;
> }
>
> +static bool set_at_max_writeback_rate(struct cache_set *c,
> + struct cached_dev *dc)
> +{
> + int i, dirty_dc_nr = 0;
> + struct bcache_device *d;
> +
> + mutex_lock(&bch_register_lock);
> + for (i = 0; i < c->devices_max_used; i++) {
> + if (!c->devices[i])
> + continue;
> + if (UUID_FLASH_ONLY(&c->uuids[i]))
> + continue;
> + d = c->devices[i];
> + dc = container_of(d, struct cached_dev, disk);
> + if (atomic_read(&dc->has_dirty))
> + dirty_dc_nr++;
> + }
> + mutex_unlock(&bch_register_lock);
> +
> + /*
> + * Idle_counter is increased everytime when update_writeback_rate()
> + * is rescheduled in. If all backing devices attached to the same
> + * cache set has same dc->writeback_rate_update_seconds value, it
> + * is about 10 rounds of update_writeback_rate() is called on each
> + * backing device, then the code will fall through at set 1 to
> + * c->at_max_writeback_rate, and a max wrteback rate to each
> + * dc->writeback_rate.rate. This is not very accurate but works well
> + * to make sure the whole cache set has no new I/O coming before
> + * writeback rate is set to a max number.
> + */
> + if (atomic_inc_return(&c->idle_counter) < dirty_dc_nr * 10)
> + return false;
> +
> + if (atomic_read(&c->at_max_writeback_rate) != 1)
> + atomic_set(&c->at_max_writeback_rate, 1);
> +
> +
> + atomic64_set(&dc->writeback_rate.rate, INT_MAX);
> +
> + /* keep writeback_rate_target as existing value */
> + dc->writeback_rate_proportional = 0;
> + dc->writeback_rate_integral_scaled = 0;
> + dc->writeback_rate_change = 0;
> +
> + /*
> + * Check c->idle_counter and c->at_max_writeback_rate agagain in case
> + * new I/O arrives during before set_at_max_writeback_rate() returns.
> + * Then the writeback rate is set to 1, and its new value should be
> + * decided via __update_writeback_rate().
> + */
> + if (atomic_read(&c->idle_counter) < dirty_dc_nr * 10 ||
> + !atomic_read(&c->at_max_writeback_rate))
> + return false;
> +
> + return true;
> +}
> +
> static void __update_writeback_rate(struct cached_dev *dc)
> {
> /*
> @@ -104,8 +161,9 @@ static void __update_writeback_rate(struct cached_dev *dc)
>
> dc->writeback_rate_proportional = proportional_scaled;
> dc->writeback_rate_integral_scaled = integral_scaled;
> - dc->writeback_rate_change = new_rate - dc->writeback_rate.rate;
> - dc->writeback_rate.rate = new_rate;
> + dc->writeback_rate_change = new_rate -
> + atomic64_read(&dc->writeback_rate.rate);
> + atomic64_set(&dc->writeback_rate.rate, new_rate);
> dc->writeback_rate_target = target;
> }
>
> @@ -138,9 +196,16 @@ static void update_writeback_rate(struct work_struct *work)
>
> down_read(&dc->writeback_lock);
>
> - if (atomic_read(&dc->has_dirty) &&
> - dc->writeback_percent)
> - __update_writeback_rate(dc);
> + if (atomic_read(&dc->has_dirty) && dc->writeback_percent) {
> + /*
> + * If the whole cache set is idle, set_at_max_writeback_rate()
> + * will set writeback rate to a max number. Then it is
> + * unncessary to update writeback rate for an idle cache set
> + * in maximum writeback rate number(s).
> + */
> + if (!set_at_max_writeback_rate(c, dc))
> + __update_writeback_rate(dc);
> + }
>
> up_read(&dc->writeback_lock);
>
> @@ -422,27 +487,6 @@ static void read_dirty(struct cached_dev *dc)
>
> delay = writeback_delay(dc, size);
>
> - /* If the control system would wait for at least half a
> - * second, and there's been no reqs hitting the backing disk
> - * for awhile: use an alternate mode where we have at most
> - * one contiguous set of writebacks in flight at a time. If
> - * someone wants to do IO it will be quick, as it will only
> - * have to contend with one operation in flight, and we'll
> - * be round-tripping data to the backing disk as quickly as
> - * it can accept it.
> - */
> - if (delay >= HZ / 2) {
> - /* 3 means at least 1.5 seconds, up to 7.5 if we
> - * have slowed way down.
> - */
> - if (atomic_inc_return(&dc->backing_idle) >= 3) {
> - /* Wait for current I/Os to finish */
> - closure_sync(&cl);
> - /* And immediately launch a new set. */
> - delay = 0;
> - }
> - }
> -
> while (!kthread_should_stop() &&
> !test_bit(CACHE_SET_IO_DISABLE, &dc->disk.c->flags) &&
> delay) {
> @@ -715,7 +759,7 @@ void bch_cached_dev_writeback_init(struct cached_dev *dc)
> dc->writeback_running = true;
> dc->writeback_percent = 10;
> dc->writeback_delay = 30;
> - dc->writeback_rate.rate = 1024;
> + atomic64_set(&dc->writeback_rate.rate, 1024);
> dc->writeback_rate_minimum = 8;
>
> dc->writeback_rate_update_seconds = WRITEBACK_RATE_UPDATE_SECS_DEFAULT;
> --
> 2.17.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bcache" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [Qemu-devel] [PATCH v3] trace/simple: fix hang in child after fork(2)
From: Stefan Hajnoczi @ 2018-07-23 13:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Cornelia Huck
In-Reply-To: <20180717101944.11691-1-stefanha@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1248 bytes --]
On Tue, Jul 17, 2018 at 11:19:44AM +0100, Stefan Hajnoczi wrote:
> The simple trace backend spawns a write-out thread which is used to
> asynchronously flush the in-memory ring buffer to disk.
>
> fork(2) does not clone all threads, only the thread that invoked
> fork(2). As a result there is no write-out thread in the child process!
>
> This causes a hang during shutdown when atexit(3) handler installed by
> the simple trace backend waits for the non-existent write-out thread.
>
> This patch uses pthread_atfork(3) to terminate the write-out thread
> before fork and restart it in both the parent and child after fork.
> This solves a hang in qemu-iotests 147 due to qemu-nbd --fork usage.
>
> Reported-by: Cornelia Huck <cohuck@redhat.com>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> v3:
> * Hold trace_lock across fork() to prevent possibility of another
> thread holding it and disappearing [Paolo]
>
> trace/simple.c | 80 ++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 71 insertions(+), 9 deletions(-)
Thanks, applied to my tracing tree for 3.0:
https://github.com/stefanha/qemu/commits/tracing
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc/powernv : Save/Restore SPRG3 on entry/exit from stop.
From: Gautham R Shenoy @ 2018-07-23 12:13 UTC (permalink / raw)
To: Greg KH; +Cc: Gautham R. Shenoy, stable, Michael Ellerman
In-Reply-To: <20180723075350.GA31879@kroah.com>
Hello Greg,
On Mon, Jul 23, 2018 at 09:53:50AM +0200, Greg KH wrote:
> On Mon, Jul 23, 2018 at 08:16:28AM +0200, Greg KH wrote:
> > On Mon, Jul 23, 2018 at 11:00:55AM +0530, Gautham R. Shenoy wrote:
> > > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> > >
> > > [ Upstream commit b03897cf318dfc47de33a7ecbc7655584266f034 ]
> > >
> > > On 64-bit servers, SPRN_SPRG3 and its userspace read-only mirror
> > > SPRN_USPRG3 are used as userspace VDSO write and read registers
> > > respectively.
> > >
> > > SPRN_SPRG3 is lost when we enter stop4 and above, and is currently not
> > > restored. As a result, any read from SPRN_USPRG3 returns zero on an
> > > exit from stop4 and above.
> > >
> > > Thus in this situation, on POWER9, any call from sched_getcpu() always
> > > returns zero, as on powerpc, we call __kernel_getcpu() which relies
> > > upon SPRN_USPRG3 to report the CPU and NUMA node information.
> > >
> > > Fix this by restoring SPRN_SPRG3 on wake up from a deep stop state
> > > with the sprg_vdso value that is cached in PACA.
> > >
> > > Fixes: e1c1cfed5432 ("powerpc/powernv: Save/Restore additional SPRs
> > > for stop4 cpuidle")
> > >
> > > Reported-by: Florian Weimer <fweimer@redhat.com>
> > > Cc: <stable@vger.kernel.org> # 4.14
> > > Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> > > Reviewed-by: Michael Ellerman <mpe@ellerman.id.au>
> > > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> > > ---
> > > arch/powerpc/kernel/idle_book3s.S | 2 ++
> > > 1 file changed, 2 insertions(+)
> >
> > What stable tree(s) do you want this applied to?
>
> I've now guessed that it is 4.14.y. If that's not the case, please let
> me know.
Yes, this backport is for 4.14.y. The upstream commit cleanly applies
on 4.15.18 and 4.16.18.
>
> greg k-h
>
--
Thanks and Regards
gautham.
^ permalink raw reply
* [Qemu-devel] [PULL 0/2] QAPI and monitor patches for 2018-07-23 (3.0.0-rc2)
From: Markus Armbruster @ 2018-07-23 13:13 UTC (permalink / raw)
To: qemu-devel
The following changes since commit 6598f0cdad6acc6674c4f060fa46e537228c2c47:
po: Don't include comments with location (2018-07-23 10:50:54 +0100)
are available in the Git repository at:
git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2018-07-23
for you to fetch changes up to 62aa1d887ff9fc76adb488d31447d126a78f4b8f:
monitor: Fix unsafe sharing of @cur_mon among threads (2018-07-23 14:00:03 +0200)
----------------------------------------------------------------
QAPI and monitor patches for 2018-07-23 (3.0.0-rc2)
This pull request consists of a late external interface tweak and a
thread safety fix, both related to QMP out-of-band execution.
----------------------------------------------------------------
Markus Armbruster (1):
qapi: Make 'allow-oob' optional in SchemaInfoCommand
Peter Xu (1):
monitor: Fix unsafe sharing of @cur_mon among threads
include/monitor/monitor.h | 2 +-
monitor.c | 2 +-
qapi/introspect.json | 6 +++---
scripts/qapi/introspect.py | 10 +++++-----
stubs/monitor.c | 2 +-
tests/test-util-sockets.c | 2 +-
6 files changed, 12 insertions(+), 12 deletions(-)
armbru@dusky:~/work/qemu$ rm *patch
armbru@dusky:~/work/qemu$ git-format-patch --subject-prefix PULL master
0000-cover-letter.patch
0001-qapi-Make-allow-oob-optional-in-SchemaInfoCommand.patch
0002-monitor-Fix-unsafe-sharing-of-cur_mon-among-threads.patch
--
2.17.1
^ permalink raw reply
* [Qemu-devel] [PULL 2/2] monitor: Fix unsafe sharing of @cur_mon among threads
From: Markus Armbruster @ 2018-07-23 13:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu
In-Reply-To: <20180723131356.27466-1-armbru@redhat.com>
From: Peter Xu <peterx@redhat.com>
@cur_mon is null unless the main thread is running monitor code, either
HMP code within monitor_read(), or QMP code within
monitor_qmp_dispatch().
Use of @cur_mon outside the main thread is therefore unsafe.
Most of its uses are in monitor command handlers. These run in the main
thread.
However, there are also uses hiding elsewhere, such as in
error_vprintf(), and thus error_report(), making these functions unsafe
outside the main thread. No such unsafe uses are known at this time.
Regardless, this is an unnecessary trap. It's an ancient trap, though.
More recently, commit cf869d53172 "qmp: support out-of-band (oob)
execution" spiced things up: the monitor I/O thread assigns to @cur_mon
when executing commands out-of-band. Having two threads save, set and
restore @cur_mon without synchronization is definitely unsafe. We can
end up with @cur_mon null while the main thread runs monitor code, or
non-null while it runs non-monitor code.
We could fix this by making the I/O thread not mess with @cur_mon, but
that would leave the trap armed and ready.
Instead, make @cur_mon thread-local. It's now reliably null unless the
thread is running monitor code.
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[peterx: update subject and commit message written by Markus]
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180720033451.32710-1-peterx@redhat.com>
---
include/monitor/monitor.h | 2 +-
monitor.c | 2 +-
stubs/monitor.c | 2 +-
tests/test-util-sockets.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index d6ab70cae2..2ef5e04b37 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -6,7 +6,7 @@
#include "qapi/qapi-types-misc.h"
#include "qemu/readline.h"
-extern Monitor *cur_mon;
+extern __thread Monitor *cur_mon;
/* flags for monitor_init */
/* 0x01 unused */
diff --git a/monitor.c b/monitor.c
index be29634a00..f75027b09e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -290,7 +290,7 @@ static mon_cmd_t info_cmds[];
QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
-Monitor *cur_mon;
+__thread Monitor *cur_mon;
static void monitor_command_cb(void *opaque, const char *cmdline,
void *readline_opaque);
diff --git a/stubs/monitor.c b/stubs/monitor.c
index e018c8f594..3890771bb5 100644
--- a/stubs/monitor.c
+++ b/stubs/monitor.c
@@ -3,7 +3,7 @@
#include "qemu-common.h"
#include "monitor/monitor.h"
-Monitor *cur_mon = NULL;
+__thread Monitor *cur_mon;
int monitor_get_fd(Monitor *mon, const char *name, Error **errp)
{
diff --git a/tests/test-util-sockets.c b/tests/test-util-sockets.c
index acadd85e8f..6195a3ac36 100644
--- a/tests/test-util-sockets.c
+++ b/tests/test-util-sockets.c
@@ -69,7 +69,7 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
* stubs/monitor.c is defined, to make sure monitor.o is discarded
* otherwise we get duplicate syms at link time.
*/
-Monitor *cur_mon;
+__thread Monitor *cur_mon;
void monitor_init(Chardev *chr, int flags) {}
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PULL 1/2] qapi: Make 'allow-oob' optional in SchemaInfoCommand
From: Markus Armbruster @ 2018-07-23 13:13 UTC (permalink / raw)
To: qemu-devel
In-Reply-To: <20180723131356.27466-1-armbru@redhat.com>
Making 'allow-oob' optional in SchemaInfoCommand permits omitting it
in the common case. Shrinks query-qmp-schema's output from 122.1KiB
to 118.6KiB for me.
Note that out-of-band execution is still experimental (you have to
configure the monitor with x-oob=on to use it).
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180718090557.17248-1-armbru@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
---
qapi/introspect.json | 6 +++---
scripts/qapi/introspect.py | 10 +++++-----
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/qapi/introspect.json b/qapi/introspect.json
index c7f67b7d78..137b39b992 100644
--- a/qapi/introspect.json
+++ b/qapi/introspect.json
@@ -259,8 +259,8 @@
#
# @ret-type: the name of the command's result type.
#
-# @allow-oob: whether the command allows out-of-band execution.
-# (Since: 2.12)
+# @allow-oob: whether the command allows out-of-band execution,
+# defaults to false (Since: 2.12)
#
# TODO: @success-response (currently irrelevant, because it's QGA, not QMP)
#
@@ -268,7 +268,7 @@
##
{ 'struct': 'SchemaInfoCommand',
'data': { 'arg-type': 'str', 'ret-type': 'str',
- 'allow-oob': 'bool' } }
+ '*allow-oob': 'bool' } }
##
# @SchemaInfoEvent:
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 70ca5dd876..189a4edaba 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -184,11 +184,11 @@ const QLitObject %(c_name)s = %(c_string)s;
success_response, boxed, allow_oob, allow_preconfig):
arg_type = arg_type or self._schema.the_empty_object_type
ret_type = ret_type or self._schema.the_empty_object_type
- self._gen_qlit(name, 'command',
- {'arg-type': self._use_type(arg_type),
- 'ret-type': self._use_type(ret_type),
- 'allow-oob': allow_oob},
- ifcond)
+ obj = {'arg-type': self._use_type(arg_type),
+ 'ret-type': self._use_type(ret_type) }
+ if allow_oob:
+ obj['allow-oob'] = allow_oob
+ self._gen_qlit(name, 'command', obj, ifcond)
def visit_event(self, name, info, ifcond, arg_type, boxed):
arg_type = arg_type or self._schema.the_empty_object_type
--
2.17.1
^ permalink raw reply related
* Re: INFO: task hung in fuse_reverse_inval_entry
From: Miklos Szeredi @ 2018-07-23 12:12 UTC (permalink / raw)
To: Dmitry Vyukov; +Cc: linux-fsdevel, LKML, syzkaller-bugs, syzbot
In-Reply-To: <CACT4Y+bKU8f4jVENYHX=fzNVd95A4vce2F=UCV12paVNFv-LNg@mail.gmail.com>
On Mon, Jul 23, 2018 at 10:11 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Mon, Jul 23, 2018 at 9:59 AM, syzbot
> <syzbot+bb6d800770577a083f8c@syzkaller.appspotmail.com> wrote:
>> Hello,
>>
>> syzbot found the following crash on:
>>
>> HEAD commit: d72e90f33aa4 Linux 4.18-rc6
>> git tree: upstream
>> console output: https://syzkaller.appspot.com/x/log.txt?x=1324f794400000
>> kernel config: https://syzkaller.appspot.com/x/.config?x=68af3495408deac5
>> dashboard link: https://syzkaller.appspot.com/bug?extid=bb6d800770577a083f8c
>> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=11564d1c400000
>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16fc570c400000
>
>
> Hi fuse maintainers,
>
> We are seeing a bunch of such deadlocks in fuse on syzbot. As far as I
> understand this is mostly working-as-intended (parts about deadlocks
> in Documentation/filesystems/fuse.txt). The intended way to resolve
> this is aborting connections via fusectl, right?
Yes. Alternative is with "umount -f".
> The doc says "Under
> the fuse control filesystem each connection has a directory named by a
> unique number". The question is: if I start a process and this process
> can mount fuse, how do I kill it? I mean: totally and certainly get
> rid of it right away? How do I find these unique numbers for the
> mounts it created?
It is the device number found in st_dev for the mount. Other than
doing stat(2) it is possible to find out the device number by reading
/proc/$PID/mountinfo (third field).
> Taking into account that there is usually no
> operator attached to each server, I wonder if kernel could somehow
> auto-abort fuse on kill?
Depends on what the fuse server is sleeping on. If it's trying to
acquire an inode lock (e.g. unlink(2)), which is classical way to
deadlock a fuse filesystem, then it will go into an uninterruptible
sleep. There's no way in which that process can be killed except to
force a release of the offending lock, which can only be done by
aborting the request that is being performed while holding that lock.
Thanks,
Miklos
^ permalink raw reply
* Re: [PATCH 1/2] t3507: add a testcase showing failure with sparse checkout
From: Ben Peart @ 2018-07-23 13:12 UTC (permalink / raw)
To: Eric Sunshine, Elijah Newren; +Cc: Git List, Junio C Hamano, Ben Peart, kewillf
In-Reply-To: <CAPig+cTjQ74gOt8gJbYUBaTRsHBvqtcmucoVdiudFt2TrhCn+g@mail.gmail.com>
On 7/21/2018 3:21 AM, Eric Sunshine wrote:
> On Sat, Jul 21, 2018 at 2:34 AM Elijah Newren <newren@gmail.com> wrote:
>> diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
>> @@ -392,4 +392,17 @@ test_expect_success 'commit --amend -s places the sign-off at the right place' '
>> +test_expect_failure 'failed cherry-pick with sparse-checkout' '
>> + pristine_detach initial &&
>> + git config core.sparseCheckout true &&
>
> Should this be test_config()?
>
I think using test_config() here is fine but...
>> + echo /unrelated >.git/info/sparse-checkout &&
>> + git read-tree --reset -u HEAD &&
>> + test_must_fail git cherry-pick -Xours picked>actual &&
>> + test_i18ngrep ! "Changes not staged for commit:" actual &&
>> + echo "/*" >.git/info/sparse-checkout &&
>> + git read-tree --reset -u HEAD &&
>> + git config core.sparseCheckout false &&
>
> See question above.
>
>> + rm .git/info/sparse-checkout
>
> Should this cleanup be done by test_when_finished()?
>
I think trying to use test_when_finished() for this really degrades the
readability of the test. See below:
test_expect_success 'failed cherry-pick with sparse-checkout' '
pristine_detach initial &&
test_config core.sparsecheckout true &&
echo /unrelated >.git/info/sparse-checkout &&
git read-tree --reset -u HEAD &&
test_when_finished "echo \"/*\" >.git/info/sparse-checkout && git
read-tree --reset -u HEAD && rm .git/info/sparse-checkout" &&
test_must_fail git cherry-pick -Xours picked>actual &&
test_i18ngrep ! "Changes not staged for commit:" actual
'
Given it takes multiple commands, I'd prefer to keep the setup and
cleanup of the sparse checkout settings symmetrical.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.