[[RESEND]PATCH staging/speakup v3 3/3] use speakup_allocate as per required context
speakup_allocate used GFP_ATOMIC for allocations even while during initialization due to it's use in notifier call. Pass GFP_ flags as well to speakup_allocate depending on the context it is called in. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/staging/speakup/main.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index 2db3f06..b811c86 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -1341,14 +1341,14 @@ static int edit_bits(struct vc_data *vc, u_char type, u_char ch, u_short key) } /* Allocation concurrency is protected by the console semaphore */ -static int speakup_allocate(struct vc_data *vc) +static int speakup_allocate(struct vc_data *vc, gfp_t gfp_flags) { int vc_num; vc_num = vc->vc_num; if (speakup_console[vc_num] == NULL) { speakup_console[vc_num] = kzalloc(sizeof(*speakup_console[0]), - GFP_ATOMIC); + gfp_flags); if (!speakup_console[vc_num]) return -ENOMEM; speakup_date(vc); @@ -2277,7 +2277,7 @@ static int vt_notifier_call(struct notifier_block *nb, switch (code) { case VT_ALLOCATE: if (vc->vc_mode == KD_TEXT) - speakup_allocate(vc); + speakup_allocate(vc, GFP_ATOMIC); break; case VT_DEALLOCATE: speakup_deallocate(vc); @@ -2362,7 +2362,7 @@ static int __init speakup_init(void) for (i = 0; i < MAX_NR_CONSOLES; i++) if (vc_cons[i].d) { - err = speakup_allocate(vc_cons[i].d); + err = speakup_allocate(vc_cons[i].d, GFP_KERNEL); if (err) goto error_kobjects; } -- 2.10.2
[[RESEND]PATCH staging/speakup v3 3/3] use speakup_allocate as per required context
speakup_allocate used GFP_ATOMIC for allocations even while during initialization due to it's use in notifier call. Pass GFP_ flags as well to speakup_allocate depending on the context it is called in. Signed-off-by: Pranay Kr. Srivastava --- drivers/staging/speakup/main.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index 2db3f06..b811c86 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -1341,14 +1341,14 @@ static int edit_bits(struct vc_data *vc, u_char type, u_char ch, u_short key) } /* Allocation concurrency is protected by the console semaphore */ -static int speakup_allocate(struct vc_data *vc) +static int speakup_allocate(struct vc_data *vc, gfp_t gfp_flags) { int vc_num; vc_num = vc->vc_num; if (speakup_console[vc_num] == NULL) { speakup_console[vc_num] = kzalloc(sizeof(*speakup_console[0]), - GFP_ATOMIC); + gfp_flags); if (!speakup_console[vc_num]) return -ENOMEM; speakup_date(vc); @@ -2277,7 +2277,7 @@ static int vt_notifier_call(struct notifier_block *nb, switch (code) { case VT_ALLOCATE: if (vc->vc_mode == KD_TEXT) - speakup_allocate(vc); + speakup_allocate(vc, GFP_ATOMIC); break; case VT_DEALLOCATE: speakup_deallocate(vc); @@ -2362,7 +2362,7 @@ static int __init speakup_init(void) for (i = 0; i < MAX_NR_CONSOLES; i++) if (vc_cons[i].d) { - err = speakup_allocate(vc_cons[i].d); + err = speakup_allocate(vc_cons[i].d, GFP_KERNEL); if (err) goto error_kobjects; } -- 2.10.2
[PATCH staging/speakup v3 2/3] remove unnecessary initial allocation of vc
This patch removes the unnecessary allocation of current foreground vc during initialization. This initialization is already handled in the loop that follows it for all available virtual consoles. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> Reviewed-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> --- drivers/staging/speakup/main.c | 11 --- 1 file changed, 11 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index a1d5b66..ca817ca 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -2317,7 +2317,6 @@ static int __init speakup_init(void) { int i; long err = 0; - struct st_spk_t *first_console; struct vc_data *vc = vc_cons[fg_console].d; struct var_t *var; @@ -2342,15 +2341,6 @@ static int __init speakup_init(void) if (err) goto error_virtkeyboard; - first_console = kzalloc(sizeof(*first_console), GFP_KERNEL); - if (!first_console) { - err = -ENOMEM; - goto error_alloc; - } - - speakup_console[vc->vc_num] = first_console; - speakup_date(vc); - for (i = 0; i < MAX_NR_CONSOLES; i++) if (vc_cons[i].d) { err = speakup_allocate(vc_cons[i].d); @@ -2412,7 +2402,6 @@ static int __init speakup_init(void) for (i = 0; i < MAX_NR_CONSOLES; i++) kfree(speakup_console[i]); -error_alloc: speakup_remove_virtual_keyboard(); error_virtkeyboard: -- 2.10.2
[PATCH staging/speakup v3 2/3] remove unnecessary initial allocation of vc
This patch removes the unnecessary allocation of current foreground vc during initialization. This initialization is already handled in the loop that follows it for all available virtual consoles. Signed-off-by: Pranay Kr. Srivastava Reviewed-by: Samuel Thibault --- drivers/staging/speakup/main.c | 11 --- 1 file changed, 11 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index a1d5b66..ca817ca 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -2317,7 +2317,6 @@ static int __init speakup_init(void) { int i; long err = 0; - struct st_spk_t *first_console; struct vc_data *vc = vc_cons[fg_console].d; struct var_t *var; @@ -2342,15 +2341,6 @@ static int __init speakup_init(void) if (err) goto error_virtkeyboard; - first_console = kzalloc(sizeof(*first_console), GFP_KERNEL); - if (!first_console) { - err = -ENOMEM; - goto error_alloc; - } - - speakup_console[vc->vc_num] = first_console; - speakup_date(vc); - for (i = 0; i < MAX_NR_CONSOLES; i++) if (vc_cons[i].d) { err = speakup_allocate(vc_cons[i].d); @@ -2412,7 +2402,6 @@ static int __init speakup_init(void) for (i = 0; i < MAX_NR_CONSOLES; i++) kfree(speakup_console[i]); -error_alloc: speakup_remove_virtual_keyboard(); error_virtkeyboard: -- 2.10.2
[PATCH staging/speakup v3 3/3] use speakup_allocate as per required context
speakup_allocate used GFP_ATOMIC for allocations even while during initialization due to it's use in notifier call. Pass GFP_ flags as well to speakup_allocate depending on the context it is called in. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/staging/speakup/main.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index ca817ca..ede842e 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -1327,14 +1327,14 @@ static int edit_bits(struct vc_data *vc, u_char type, u_char ch, u_short key) } /* Allocation concurrency is protected by the console semaphore */ -static int speakup_allocate(struct vc_data *vc) +static int speakup_allocate(struct vc_data *vc, gfp_t gfp_flags) { int vc_num; vc_num = vc->vc_num; if (speakup_console[vc_num] == NULL) { speakup_console[vc_num] = kzalloc(sizeof(*speakup_console[0]), - GFP_ATOMIC); + gfp_flags); if (speakup_console[vc_num] == NULL) return -ENOMEM; speakup_date(vc); @@ -2257,7 +2257,7 @@ static int vt_notifier_call(struct notifier_block *nb, switch (code) { case VT_ALLOCATE: if (vc->vc_mode == KD_TEXT) - speakup_allocate(vc); + speakup_allocate(vc, GFP_ATOMIC); break; case VT_DEALLOCATE: speakup_deallocate(vc); @@ -2343,7 +2343,7 @@ static int __init speakup_init(void) for (i = 0; i < MAX_NR_CONSOLES; i++) if (vc_cons[i].d) { - err = speakup_allocate(vc_cons[i].d); + err = speakup_allocate(vc_cons[i].d, GFP_KERNEL); if (err) goto error_kobjects; } -- 2.10.2
[PATCH staging/speakup v3 3/3] use speakup_allocate as per required context
speakup_allocate used GFP_ATOMIC for allocations even while during initialization due to it's use in notifier call. Pass GFP_ flags as well to speakup_allocate depending on the context it is called in. Signed-off-by: Pranay Kr. Srivastava --- drivers/staging/speakup/main.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index ca817ca..ede842e 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -1327,14 +1327,14 @@ static int edit_bits(struct vc_data *vc, u_char type, u_char ch, u_short key) } /* Allocation concurrency is protected by the console semaphore */ -static int speakup_allocate(struct vc_data *vc) +static int speakup_allocate(struct vc_data *vc, gfp_t gfp_flags) { int vc_num; vc_num = vc->vc_num; if (speakup_console[vc_num] == NULL) { speakup_console[vc_num] = kzalloc(sizeof(*speakup_console[0]), - GFP_ATOMIC); + gfp_flags); if (speakup_console[vc_num] == NULL) return -ENOMEM; speakup_date(vc); @@ -2257,7 +2257,7 @@ static int vt_notifier_call(struct notifier_block *nb, switch (code) { case VT_ALLOCATE: if (vc->vc_mode == KD_TEXT) - speakup_allocate(vc); + speakup_allocate(vc, GFP_ATOMIC); break; case VT_DEALLOCATE: speakup_deallocate(vc); @@ -2343,7 +2343,7 @@ static int __init speakup_init(void) for (i = 0; i < MAX_NR_CONSOLES; i++) if (vc_cons[i].d) { - err = speakup_allocate(vc_cons[i].d); + err = speakup_allocate(vc_cons[i].d, GFP_KERNEL); if (err) goto error_kobjects; } -- 2.10.2
[PATCH staging/speakup v3 0/3] cleanup error and initilization
Changelog from v2: Fixed the message subject line. Changelog from v1: 1. fixed kbuild warning for i386 build as reported by kbuild robot 2. split initialization code in two patches. Pranay Kr. Srivastava (3): return same error value from spk_set_key_info remove unecessary initial allocation of vc use speakup_allocate as per required context drivers/staging/speakup/main.c | 46 +- 1 file changed, 23 insertions(+), 23 deletions(-) -- 2.10.2
[PATCH staging/speakup v3 0/3] cleanup error and initilization
Changelog from v2: Fixed the message subject line. Changelog from v1: 1. fixed kbuild warning for i386 build as reported by kbuild robot 2. split initialization code in two patches. Pranay Kr. Srivastava (3): return same error value from spk_set_key_info remove unecessary initial allocation of vc use speakup_allocate as per required context drivers/staging/speakup/main.c | 46 +- 1 file changed, 23 insertions(+), 23 deletions(-) -- 2.10.2
[PATCH staging/speakup v3 1/3] return same error value from spk_set_key_info
This patch makes spk_set_key_info return -EINVAL in case of failure instead of returning 4 different values for the type of error that occurred. Print the offending values instead as debug message. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/staging/speakup/main.c | 27 +++ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index c2f70ef..a1d5b66 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -1216,13 +1216,19 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer) u_char ch, version, num_keys; version = *cp++; - if (version != KEY_MAP_VER) - return -1; + if (version != KEY_MAP_VER) { + pr_debug("version found %d should be %d\n", +version, KEY_MAP_VER); + return -EINVAL; + } num_keys = *cp; states = (int)cp[1]; key_data_len = (states + 1) * (num_keys + 1); - if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) - return -2; + if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) { + pr_debug("too many key_infos (%d over %u)\n", +key_data_len + SHIFT_TBL_SIZE + 4, (unsigned int)(sizeof(spk_key_buf))); + return -EINVAL; + } memset(k_buffer, 0, SHIFT_TBL_SIZE); memset(spk_our_keys, 0, sizeof(spk_our_keys)); spk_shift_table = k_buffer; @@ -1233,14 +1239,19 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer) cp1 += 2; /* now pointing at shift states */ for (i = 1; i <= states; i++) { ch = *cp1++; - if (ch >= SHIFT_TBL_SIZE) - return -3; + if (ch >= SHIFT_TBL_SIZE) { + pr_debug("(%d) not valid shift state (max_allowed = %d)\n", ch, +SHIFT_TBL_SIZE); + return -EINVAL; + } spk_shift_table[ch] = i; } keymap_flags = *cp1++; while ((ch = *cp1)) { - if (ch >= MAX_KEY) - return -4; + if (ch >= MAX_KEY) { + pr_debug("(%d), not valid key, (max_allowed = %d)\n", ch, MAX_KEY); + return -EINVAL; + } spk_our_keys[ch] = cp1; cp1 += states + 1; } -- 2.10.2
[PATCH staging/speakup v3 1/3] return same error value from spk_set_key_info
This patch makes spk_set_key_info return -EINVAL in case of failure instead of returning 4 different values for the type of error that occurred. Print the offending values instead as debug message. Signed-off-by: Pranay Kr. Srivastava --- drivers/staging/speakup/main.c | 27 +++ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index c2f70ef..a1d5b66 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -1216,13 +1216,19 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer) u_char ch, version, num_keys; version = *cp++; - if (version != KEY_MAP_VER) - return -1; + if (version != KEY_MAP_VER) { + pr_debug("version found %d should be %d\n", +version, KEY_MAP_VER); + return -EINVAL; + } num_keys = *cp; states = (int)cp[1]; key_data_len = (states + 1) * (num_keys + 1); - if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) - return -2; + if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) { + pr_debug("too many key_infos (%d over %u)\n", +key_data_len + SHIFT_TBL_SIZE + 4, (unsigned int)(sizeof(spk_key_buf))); + return -EINVAL; + } memset(k_buffer, 0, SHIFT_TBL_SIZE); memset(spk_our_keys, 0, sizeof(spk_our_keys)); spk_shift_table = k_buffer; @@ -1233,14 +1239,19 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer) cp1 += 2; /* now pointing at shift states */ for (i = 1; i <= states; i++) { ch = *cp1++; - if (ch >= SHIFT_TBL_SIZE) - return -3; + if (ch >= SHIFT_TBL_SIZE) { + pr_debug("(%d) not valid shift state (max_allowed = %d)\n", ch, +SHIFT_TBL_SIZE); + return -EINVAL; + } spk_shift_table[ch] = i; } keymap_flags = *cp1++; while ((ch = *cp1)) { - if (ch >= MAX_KEY) - return -4; + if (ch >= MAX_KEY) { + pr_debug("(%d), not valid key, (max_allowed = %d)\n", ch, MAX_KEY); + return -EINVAL; + } spk_our_keys[ch] = cp1; cp1 += states + 1; } -- 2.10.2
[PATCH SPEAKUP v2 2/3] remove unecessary initial allocation of vc
This patch removes the unnecessary allocation of current foreground vc during initialization. This initialization is already handled in the loop that follows it for all available virtual consoles. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/staging/speakup/main.c | 11 --- 1 file changed, 11 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index a1d5b66..ca817ca 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -2317,7 +2317,6 @@ static int __init speakup_init(void) { int i; long err = 0; - struct st_spk_t *first_console; struct vc_data *vc = vc_cons[fg_console].d; struct var_t *var; @@ -2342,15 +2341,6 @@ static int __init speakup_init(void) if (err) goto error_virtkeyboard; - first_console = kzalloc(sizeof(*first_console), GFP_KERNEL); - if (!first_console) { - err = -ENOMEM; - goto error_alloc; - } - - speakup_console[vc->vc_num] = first_console; - speakup_date(vc); - for (i = 0; i < MAX_NR_CONSOLES; i++) if (vc_cons[i].d) { err = speakup_allocate(vc_cons[i].d); @@ -2412,7 +2402,6 @@ static int __init speakup_init(void) for (i = 0; i < MAX_NR_CONSOLES; i++) kfree(speakup_console[i]); -error_alloc: speakup_remove_virtual_keyboard(); error_virtkeyboard: -- 2.10.2
[PATCH SPEAKUP v2 2/3] remove unecessary initial allocation of vc
This patch removes the unnecessary allocation of current foreground vc during initialization. This initialization is already handled in the loop that follows it for all available virtual consoles. Signed-off-by: Pranay Kr. Srivastava --- drivers/staging/speakup/main.c | 11 --- 1 file changed, 11 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index a1d5b66..ca817ca 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -2317,7 +2317,6 @@ static int __init speakup_init(void) { int i; long err = 0; - struct st_spk_t *first_console; struct vc_data *vc = vc_cons[fg_console].d; struct var_t *var; @@ -2342,15 +2341,6 @@ static int __init speakup_init(void) if (err) goto error_virtkeyboard; - first_console = kzalloc(sizeof(*first_console), GFP_KERNEL); - if (!first_console) { - err = -ENOMEM; - goto error_alloc; - } - - speakup_console[vc->vc_num] = first_console; - speakup_date(vc); - for (i = 0; i < MAX_NR_CONSOLES; i++) if (vc_cons[i].d) { err = speakup_allocate(vc_cons[i].d); @@ -2412,7 +2402,6 @@ static int __init speakup_init(void) for (i = 0; i < MAX_NR_CONSOLES; i++) kfree(speakup_console[i]); -error_alloc: speakup_remove_virtual_keyboard(); error_virtkeyboard: -- 2.10.2
[PATCH SPEAKUP v2 3/3] use spkeaup_allocate as per required context
speakup_allocate used GFP_ATOMIC for allocations even while during initialization due to it's use in notifier call. Pass GFP_ flags as well to speakup_allocate depending on the context it is called in. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/staging/speakup/main.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index ca817ca..ede842e 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -1327,14 +1327,14 @@ static int edit_bits(struct vc_data *vc, u_char type, u_char ch, u_short key) } /* Allocation concurrency is protected by the console semaphore */ -static int speakup_allocate(struct vc_data *vc) +static int speakup_allocate(struct vc_data *vc, gfp_t gfp_flags) { int vc_num; vc_num = vc->vc_num; if (speakup_console[vc_num] == NULL) { speakup_console[vc_num] = kzalloc(sizeof(*speakup_console[0]), - GFP_ATOMIC); + gfp_flags); if (speakup_console[vc_num] == NULL) return -ENOMEM; speakup_date(vc); @@ -2257,7 +2257,7 @@ static int vt_notifier_call(struct notifier_block *nb, switch (code) { case VT_ALLOCATE: if (vc->vc_mode == KD_TEXT) - speakup_allocate(vc); + speakup_allocate(vc, GFP_ATOMIC); break; case VT_DEALLOCATE: speakup_deallocate(vc); @@ -2343,7 +2343,7 @@ static int __init speakup_init(void) for (i = 0; i < MAX_NR_CONSOLES; i++) if (vc_cons[i].d) { - err = speakup_allocate(vc_cons[i].d); + err = speakup_allocate(vc_cons[i].d, GFP_KERNEL); if (err) goto error_kobjects; } -- 2.10.2
[PATCH SPEAKUP v2 0/3] cleanup error and initilization
Changelog from v1: 1. fixed kbuild warning for i386 build as reported by kbuild robot 2. split initialization code in two patches. Pranay Kr. Srivastava (3): return same error value from spk_set_key_info remove unecessary initial allocation of vc use spkeaup_allocate as per required context drivers/staging/speakup/main.c | 46 +- 1 file changed, 23 insertions(+), 23 deletions(-) -- 2.10.2
[PATCH SPEAKUP v2 3/3] use spkeaup_allocate as per required context
speakup_allocate used GFP_ATOMIC for allocations even while during initialization due to it's use in notifier call. Pass GFP_ flags as well to speakup_allocate depending on the context it is called in. Signed-off-by: Pranay Kr. Srivastava --- drivers/staging/speakup/main.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index ca817ca..ede842e 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -1327,14 +1327,14 @@ static int edit_bits(struct vc_data *vc, u_char type, u_char ch, u_short key) } /* Allocation concurrency is protected by the console semaphore */ -static int speakup_allocate(struct vc_data *vc) +static int speakup_allocate(struct vc_data *vc, gfp_t gfp_flags) { int vc_num; vc_num = vc->vc_num; if (speakup_console[vc_num] == NULL) { speakup_console[vc_num] = kzalloc(sizeof(*speakup_console[0]), - GFP_ATOMIC); + gfp_flags); if (speakup_console[vc_num] == NULL) return -ENOMEM; speakup_date(vc); @@ -2257,7 +2257,7 @@ static int vt_notifier_call(struct notifier_block *nb, switch (code) { case VT_ALLOCATE: if (vc->vc_mode == KD_TEXT) - speakup_allocate(vc); + speakup_allocate(vc, GFP_ATOMIC); break; case VT_DEALLOCATE: speakup_deallocate(vc); @@ -2343,7 +2343,7 @@ static int __init speakup_init(void) for (i = 0; i < MAX_NR_CONSOLES; i++) if (vc_cons[i].d) { - err = speakup_allocate(vc_cons[i].d); + err = speakup_allocate(vc_cons[i].d, GFP_KERNEL); if (err) goto error_kobjects; } -- 2.10.2
[PATCH SPEAKUP v2 0/3] cleanup error and initilization
Changelog from v1: 1. fixed kbuild warning for i386 build as reported by kbuild robot 2. split initialization code in two patches. Pranay Kr. Srivastava (3): return same error value from spk_set_key_info remove unecessary initial allocation of vc use spkeaup_allocate as per required context drivers/staging/speakup/main.c | 46 +- 1 file changed, 23 insertions(+), 23 deletions(-) -- 2.10.2
[PATCH SPEAKUP v2 1/3] return same error value from spk_set_key_info
This patch makes spk_set_key_info return -EINVAL in case of failure instead of returning 4 different values for the type of error that occurred. Print the offending values instead as debug message. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/staging/speakup/main.c | 27 +++ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index c2f70ef..a1d5b66 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -1216,13 +1216,19 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer) u_char ch, version, num_keys; version = *cp++; - if (version != KEY_MAP_VER) - return -1; + if (version != KEY_MAP_VER) { + pr_debug("version found %d should be %d\n", +version, KEY_MAP_VER); + return -EINVAL; + } num_keys = *cp; states = (int)cp[1]; key_data_len = (states + 1) * (num_keys + 1); - if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) - return -2; + if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) { + pr_debug("too many key_infos (%d over %u)\n", +key_data_len + SHIFT_TBL_SIZE + 4, (unsigned int)(sizeof(spk_key_buf))); + return -EINVAL; + } memset(k_buffer, 0, SHIFT_TBL_SIZE); memset(spk_our_keys, 0, sizeof(spk_our_keys)); spk_shift_table = k_buffer; @@ -1233,14 +1239,19 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer) cp1 += 2; /* now pointing at shift states */ for (i = 1; i <= states; i++) { ch = *cp1++; - if (ch >= SHIFT_TBL_SIZE) - return -3; + if (ch >= SHIFT_TBL_SIZE) { + pr_debug("(%d) not valid shift state (max_allowed = %d)\n", ch, +SHIFT_TBL_SIZE); + return -EINVAL; + } spk_shift_table[ch] = i; } keymap_flags = *cp1++; while ((ch = *cp1)) { - if (ch >= MAX_KEY) - return -4; + if (ch >= MAX_KEY) { + pr_debug("(%d), not valid key, (max_allowed = %d)\n", ch, MAX_KEY); + return -EINVAL; + } spk_our_keys[ch] = cp1; cp1 += states + 1; } -- 2.10.2
[PATCH SPEAKUP v2 1/3] return same error value from spk_set_key_info
This patch makes spk_set_key_info return -EINVAL in case of failure instead of returning 4 different values for the type of error that occurred. Print the offending values instead as debug message. Signed-off-by: Pranay Kr. Srivastava --- drivers/staging/speakup/main.c | 27 +++ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index c2f70ef..a1d5b66 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -1216,13 +1216,19 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer) u_char ch, version, num_keys; version = *cp++; - if (version != KEY_MAP_VER) - return -1; + if (version != KEY_MAP_VER) { + pr_debug("version found %d should be %d\n", +version, KEY_MAP_VER); + return -EINVAL; + } num_keys = *cp; states = (int)cp[1]; key_data_len = (states + 1) * (num_keys + 1); - if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) - return -2; + if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) { + pr_debug("too many key_infos (%d over %u)\n", +key_data_len + SHIFT_TBL_SIZE + 4, (unsigned int)(sizeof(spk_key_buf))); + return -EINVAL; + } memset(k_buffer, 0, SHIFT_TBL_SIZE); memset(spk_our_keys, 0, sizeof(spk_our_keys)); spk_shift_table = k_buffer; @@ -1233,14 +1239,19 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer) cp1 += 2; /* now pointing at shift states */ for (i = 1; i <= states; i++) { ch = *cp1++; - if (ch >= SHIFT_TBL_SIZE) - return -3; + if (ch >= SHIFT_TBL_SIZE) { + pr_debug("(%d) not valid shift state (max_allowed = %d)\n", ch, +SHIFT_TBL_SIZE); + return -EINVAL; + } spk_shift_table[ch] = i; } keymap_flags = *cp1++; while ((ch = *cp1)) { - if (ch >= MAX_KEY) - return -4; + if (ch >= MAX_KEY) { + pr_debug("(%d), not valid key, (max_allowed = %d)\n", ch, MAX_KEY); + return -EINVAL; + } spk_our_keys[ch] = cp1; cp1 += states + 1; } -- 2.10.2
[PATCH-SPEAKUP 2/2] remove unecessary initial allocation of vc
This patch removes the unnecessary allocation of current foreground vc during initialization. This initialization is already handled in the loop that follows it for all available virtual consoles. Also change the prototype of speakup_allocate to take extra argument of gfp_* flags. Thus not requiring GFP_ATOMIC during initialization. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/staging/speakup/main.c | 19 --- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index 6667cf2..f81a936 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -1327,14 +1327,14 @@ static int edit_bits(struct vc_data *vc, u_char type, u_char ch, u_short key) } /* Allocation concurrency is protected by the console semaphore */ -static int speakup_allocate(struct vc_data *vc) +static int speakup_allocate(struct vc_data *vc, gfp_t gfp_flags) { int vc_num; vc_num = vc->vc_num; if (speakup_console[vc_num] == NULL) { speakup_console[vc_num] = kzalloc(sizeof(*speakup_console[0]), - GFP_ATOMIC); + gfp_flags); if (speakup_console[vc_num] == NULL) return -ENOMEM; speakup_date(vc); @@ -2257,7 +2257,7 @@ static int vt_notifier_call(struct notifier_block *nb, switch (code) { case VT_ALLOCATE: if (vc->vc_mode == KD_TEXT) - speakup_allocate(vc); + speakup_allocate(vc, GFP_ATOMIC); break; case VT_DEALLOCATE: speakup_deallocate(vc); @@ -2317,7 +2317,6 @@ static int __init speakup_init(void) { int i; long err = 0; - struct st_spk_t *first_console; struct vc_data *vc = vc_cons[fg_console].d; struct var_t *var; @@ -2342,18 +2341,9 @@ static int __init speakup_init(void) if (err) goto error_virtkeyboard; - first_console = kzalloc(sizeof(*first_console), GFP_KERNEL); - if (!first_console) { - err = -ENOMEM; - goto error_alloc; - } - - speakup_console[vc->vc_num] = first_console; - speakup_date(vc); - for (i = 0; i < MAX_NR_CONSOLES; i++) if (vc_cons[i].d) { - err = speakup_allocate(vc_cons[i].d); + err = speakup_allocate(vc_cons[i].d, GFP_KERNEL); if (err) goto error_kobjects; } @@ -2412,7 +2402,6 @@ static int __init speakup_init(void) for (i = 0; i < MAX_NR_CONSOLES; i++) kfree(speakup_console[i]); -error_alloc: speakup_remove_virtual_keyboard(); error_virtkeyboard: -- 2.10.2
[PATCH-SPEAKUP 1/2] return same error value from spk_set_key_info
This patch makes spk_set_key_info return -EINVAL in case of failure instead of returning 4 different values for the type of error that occurred. Print the offending values instead as debug message. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/staging/speakup/main.c | 27 +++ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index c2f70ef..6667cf2 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -1216,13 +1216,19 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer) u_char ch, version, num_keys; version = *cp++; - if (version != KEY_MAP_VER) - return -1; + if (version != KEY_MAP_VER) { + pr_debug("version found %d should be %d\n", +version, KEY_MAP_VER); + return -EINVAL; + } num_keys = *cp; states = (int)cp[1]; key_data_len = (states + 1) * (num_keys + 1); - if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) - return -2; + if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) { + pr_debug("too many key_infos (%d over %lu)\n", +key_data_len + SHIFT_TBL_SIZE + 4, sizeof(spk_key_buf)); + return -EINVAL; + } memset(k_buffer, 0, SHIFT_TBL_SIZE); memset(spk_our_keys, 0, sizeof(spk_our_keys)); spk_shift_table = k_buffer; @@ -1233,14 +1239,19 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer) cp1 += 2; /* now pointing at shift states */ for (i = 1; i <= states; i++) { ch = *cp1++; - if (ch >= SHIFT_TBL_SIZE) - return -3; + if (ch >= SHIFT_TBL_SIZE) { + pr_debug("(%d) not valid shift state (max_allowed = %d)\n", ch, +SHIFT_TBL_SIZE); + return -EINVAL; + } spk_shift_table[ch] = i; } keymap_flags = *cp1++; while ((ch = *cp1)) { - if (ch >= MAX_KEY) - return -4; + if (ch >= MAX_KEY) { + pr_debug("(%d), not valid key, (max_allowed = %d)\n", ch, MAX_KEY); + return -EINVAL; + } spk_our_keys[ch] = cp1; cp1 += states + 1; } -- 2.10.2
[PATCH-SPEAKUP 1/2] return same error value from spk_set_key_info
This patch makes spk_set_key_info return -EINVAL in case of failure instead of returning 4 different values for the type of error that occurred. Print the offending values instead as debug message. Signed-off-by: Pranay Kr. Srivastava --- drivers/staging/speakup/main.c | 27 +++ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index c2f70ef..6667cf2 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -1216,13 +1216,19 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer) u_char ch, version, num_keys; version = *cp++; - if (version != KEY_MAP_VER) - return -1; + if (version != KEY_MAP_VER) { + pr_debug("version found %d should be %d\n", +version, KEY_MAP_VER); + return -EINVAL; + } num_keys = *cp; states = (int)cp[1]; key_data_len = (states + 1) * (num_keys + 1); - if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) - return -2; + if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) { + pr_debug("too many key_infos (%d over %lu)\n", +key_data_len + SHIFT_TBL_SIZE + 4, sizeof(spk_key_buf)); + return -EINVAL; + } memset(k_buffer, 0, SHIFT_TBL_SIZE); memset(spk_our_keys, 0, sizeof(spk_our_keys)); spk_shift_table = k_buffer; @@ -1233,14 +1239,19 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer) cp1 += 2; /* now pointing at shift states */ for (i = 1; i <= states; i++) { ch = *cp1++; - if (ch >= SHIFT_TBL_SIZE) - return -3; + if (ch >= SHIFT_TBL_SIZE) { + pr_debug("(%d) not valid shift state (max_allowed = %d)\n", ch, +SHIFT_TBL_SIZE); + return -EINVAL; + } spk_shift_table[ch] = i; } keymap_flags = *cp1++; while ((ch = *cp1)) { - if (ch >= MAX_KEY) - return -4; + if (ch >= MAX_KEY) { + pr_debug("(%d), not valid key, (max_allowed = %d)\n", ch, MAX_KEY); + return -EINVAL; + } spk_our_keys[ch] = cp1; cp1 += states + 1; } -- 2.10.2
[PATCH-SPEAKUP 2/2] remove unecessary initial allocation of vc
This patch removes the unnecessary allocation of current foreground vc during initialization. This initialization is already handled in the loop that follows it for all available virtual consoles. Also change the prototype of speakup_allocate to take extra argument of gfp_* flags. Thus not requiring GFP_ATOMIC during initialization. Signed-off-by: Pranay Kr. Srivastava --- drivers/staging/speakup/main.c | 19 --- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index 6667cf2..f81a936 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -1327,14 +1327,14 @@ static int edit_bits(struct vc_data *vc, u_char type, u_char ch, u_short key) } /* Allocation concurrency is protected by the console semaphore */ -static int speakup_allocate(struct vc_data *vc) +static int speakup_allocate(struct vc_data *vc, gfp_t gfp_flags) { int vc_num; vc_num = vc->vc_num; if (speakup_console[vc_num] == NULL) { speakup_console[vc_num] = kzalloc(sizeof(*speakup_console[0]), - GFP_ATOMIC); + gfp_flags); if (speakup_console[vc_num] == NULL) return -ENOMEM; speakup_date(vc); @@ -2257,7 +2257,7 @@ static int vt_notifier_call(struct notifier_block *nb, switch (code) { case VT_ALLOCATE: if (vc->vc_mode == KD_TEXT) - speakup_allocate(vc); + speakup_allocate(vc, GFP_ATOMIC); break; case VT_DEALLOCATE: speakup_deallocate(vc); @@ -2317,7 +2317,6 @@ static int __init speakup_init(void) { int i; long err = 0; - struct st_spk_t *first_console; struct vc_data *vc = vc_cons[fg_console].d; struct var_t *var; @@ -2342,18 +2341,9 @@ static int __init speakup_init(void) if (err) goto error_virtkeyboard; - first_console = kzalloc(sizeof(*first_console), GFP_KERNEL); - if (!first_console) { - err = -ENOMEM; - goto error_alloc; - } - - speakup_console[vc->vc_num] = first_console; - speakup_date(vc); - for (i = 0; i < MAX_NR_CONSOLES; i++) if (vc_cons[i].d) { - err = speakup_allocate(vc_cons[i].d); + err = speakup_allocate(vc_cons[i].d, GFP_KERNEL); if (err) goto error_kobjects; } @@ -2412,7 +2402,6 @@ static int __init speakup_init(void) for (i = 0; i < MAX_NR_CONSOLES; i++) kfree(speakup_console[i]); -error_alloc: speakup_remove_virtual_keyboard(); error_virtkeyboard: -- 2.10.2
[no subject]
Sending both patches as series instead. Made the required changes as suggessted in earlier versions and fixed the warnings reported from kbuild test robot.
[no subject]
Sending both patches as series instead. Made the required changes as suggessted in earlier versions and fixed the warnings reported from kbuild test robot.
[PATCH] remove unnecessary initial allocation of vc
This patch removes the unnecessary allocation of current foreground vc during initialization. Also change the prototype of speakup_allocate to take extra argument of gfp_* flags. Thus not requiring GFP_ATOMIC during initialization. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/staging/speakup/main.c | 19 --- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index ad95905..4437b39 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -1328,14 +1328,14 @@ static int edit_bits(struct vc_data *vc, u_char type, u_char ch, u_short key) } /* Allocation concurrency is protected by the console semaphore */ -static int speakup_allocate(struct vc_data *vc) +static int speakup_allocate(struct vc_data *vc, gfp_t gfp_flags) { int vc_num; vc_num = vc->vc_num; if (speakup_console[vc_num] == NULL) { speakup_console[vc_num] = kzalloc(sizeof(*speakup_console[0]), - GFP_ATOMIC); + gfp_flags); if (speakup_console[vc_num] == NULL) return -ENOMEM; speakup_date(vc); @@ -2258,7 +2258,7 @@ static int vt_notifier_call(struct notifier_block *nb, switch (code) { case VT_ALLOCATE: if (vc->vc_mode == KD_TEXT) - speakup_allocate(vc); + speakup_allocate(vc, GFP_ATOMIC); break; case VT_DEALLOCATE: speakup_deallocate(vc); @@ -2318,7 +2318,6 @@ static int __init speakup_init(void) { int i; long err = 0; - struct st_spk_t *first_console; struct vc_data *vc = vc_cons[fg_console].d; struct var_t *var; @@ -2343,18 +2342,9 @@ static int __init speakup_init(void) if (err) goto error_virtkeyboard; - first_console = kzalloc(sizeof(*first_console), GFP_KERNEL); - if (!first_console) { - err = -ENOMEM; - goto error_alloc; - } - - speakup_console[vc->vc_num] = first_console; - speakup_date(vc); - for (i = 0; i < MAX_NR_CONSOLES; i++) if (vc_cons[i].d) { - err = speakup_allocate(vc_cons[i].d); + err = speakup_allocate(vc_cons[i].d, GFP_KERNEL); if (err) goto error_kobjects; } @@ -2413,7 +2403,6 @@ static int __init speakup_init(void) for (i = 0; i < MAX_NR_CONSOLES; i++) kfree(speakup_console[i]); -error_alloc: speakup_remove_virtual_keyboard(); error_virtkeyboard: -- 2.10.2
[PATCH] remove unnecessary initial allocation of vc
This patch removes the unnecessary allocation of current foreground vc during initialization. Also change the prototype of speakup_allocate to take extra argument of gfp_* flags. Thus not requiring GFP_ATOMIC during initialization. Signed-off-by: Pranay Kr. Srivastava --- drivers/staging/speakup/main.c | 19 --- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index ad95905..4437b39 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -1328,14 +1328,14 @@ static int edit_bits(struct vc_data *vc, u_char type, u_char ch, u_short key) } /* Allocation concurrency is protected by the console semaphore */ -static int speakup_allocate(struct vc_data *vc) +static int speakup_allocate(struct vc_data *vc, gfp_t gfp_flags) { int vc_num; vc_num = vc->vc_num; if (speakup_console[vc_num] == NULL) { speakup_console[vc_num] = kzalloc(sizeof(*speakup_console[0]), - GFP_ATOMIC); + gfp_flags); if (speakup_console[vc_num] == NULL) return -ENOMEM; speakup_date(vc); @@ -2258,7 +2258,7 @@ static int vt_notifier_call(struct notifier_block *nb, switch (code) { case VT_ALLOCATE: if (vc->vc_mode == KD_TEXT) - speakup_allocate(vc); + speakup_allocate(vc, GFP_ATOMIC); break; case VT_DEALLOCATE: speakup_deallocate(vc); @@ -2318,7 +2318,6 @@ static int __init speakup_init(void) { int i; long err = 0; - struct st_spk_t *first_console; struct vc_data *vc = vc_cons[fg_console].d; struct var_t *var; @@ -2343,18 +2342,9 @@ static int __init speakup_init(void) if (err) goto error_virtkeyboard; - first_console = kzalloc(sizeof(*first_console), GFP_KERNEL); - if (!first_console) { - err = -ENOMEM; - goto error_alloc; - } - - speakup_console[vc->vc_num] = first_console; - speakup_date(vc); - for (i = 0; i < MAX_NR_CONSOLES; i++) if (vc_cons[i].d) { - err = speakup_allocate(vc_cons[i].d); + err = speakup_allocate(vc_cons[i].d, GFP_KERNEL); if (err) goto error_kobjects; } @@ -2413,7 +2403,6 @@ static int __init speakup_init(void) for (i = 0; i < MAX_NR_CONSOLES; i++) kfree(speakup_console[i]); -error_alloc: speakup_remove_virtual_keyboard(); error_virtkeyboard: -- 2.10.2
[PATCH Speakup v2] return same error value from spk_set_key_info
This patch makes spk_set_key_info return -EINVAL in case of failure instead of returning 4 different values for the type of error that occurred. Print the offending values instead as debug message. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/staging/speakup/main.c | 28 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index c2f70ef..ad95905 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -1216,13 +1216,20 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer) u_char ch, version, num_keys; version = *cp++; - if (version != KEY_MAP_VER) - return -1; + if (version != KEY_MAP_VER) { + pr_debug("version found %d should be %d\n", +version, KEY_MAP_VER); + return -EINVAL; + } num_keys = *cp; states = (int)cp[1]; key_data_len = (states + 1) * (num_keys + 1); - if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) - return -2; + if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) { + pr_debug("key_data_len = %d, SHIFT_TBL_SIZE + 4 = %d,\t" +"sizeof(spk_key_buf) = %lu\n", key_data_len, +SHIFT_TBL_SIZE + 4, sizeof(spk_key_buf)); + return -EINVAL; + } memset(k_buffer, 0, SHIFT_TBL_SIZE); memset(spk_our_keys, 0, sizeof(spk_our_keys)); spk_shift_table = k_buffer; @@ -1233,14 +1240,19 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer) cp1 += 2; /* now pointing at shift states */ for (i = 1; i <= states; i++) { ch = *cp1++; - if (ch >= SHIFT_TBL_SIZE) - return -3; + if (ch >= SHIFT_TBL_SIZE) { + pr_debug("ch = %d, SHIFT_TBL_SIZE = %d\n", ch, +SHIFT_TBL_SIZE); + return -EINVAL; + } spk_shift_table[ch] = i; } keymap_flags = *cp1++; while ((ch = *cp1)) { - if (ch >= MAX_KEY) - return -4; + if (ch >= MAX_KEY) { + pr_debug("ch = %d, MAX_KEY = %d\n", ch, MAX_KEY); + return -EINVAL; + } spk_our_keys[ch] = cp1; cp1 += states + 1; } -- 2.10.2
[PATCH Speakup v2] return same error value from spk_set_key_info
This patch makes spk_set_key_info return -EINVAL in case of failure instead of returning 4 different values for the type of error that occurred. Print the offending values instead as debug message. Signed-off-by: Pranay Kr. Srivastava --- drivers/staging/speakup/main.c | 28 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index c2f70ef..ad95905 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -1216,13 +1216,20 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer) u_char ch, version, num_keys; version = *cp++; - if (version != KEY_MAP_VER) - return -1; + if (version != KEY_MAP_VER) { + pr_debug("version found %d should be %d\n", +version, KEY_MAP_VER); + return -EINVAL; + } num_keys = *cp; states = (int)cp[1]; key_data_len = (states + 1) * (num_keys + 1); - if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) - return -2; + if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) { + pr_debug("key_data_len = %d, SHIFT_TBL_SIZE + 4 = %d,\t" +"sizeof(spk_key_buf) = %lu\n", key_data_len, +SHIFT_TBL_SIZE + 4, sizeof(spk_key_buf)); + return -EINVAL; + } memset(k_buffer, 0, SHIFT_TBL_SIZE); memset(spk_our_keys, 0, sizeof(spk_our_keys)); spk_shift_table = k_buffer; @@ -1233,14 +1240,19 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer) cp1 += 2; /* now pointing at shift states */ for (i = 1; i <= states; i++) { ch = *cp1++; - if (ch >= SHIFT_TBL_SIZE) - return -3; + if (ch >= SHIFT_TBL_SIZE) { + pr_debug("ch = %d, SHIFT_TBL_SIZE = %d\n", ch, +SHIFT_TBL_SIZE); + return -EINVAL; + } spk_shift_table[ch] = i; } keymap_flags = *cp1++; while ((ch = *cp1)) { - if (ch >= MAX_KEY) - return -4; + if (ch >= MAX_KEY) { + pr_debug("ch = %d, MAX_KEY = %d\n", ch, MAX_KEY); + return -EINVAL; + } spk_our_keys[ch] = cp1; cp1 += states + 1; } -- 2.10.2
[PATCH] return same error value from spk_set_key_info
This patch makes spk_set_key_info return -EINVAL in case of failure instead of returning 4 different values for the type of error that occurred. Retain the previous error values as debug messages instead. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/staging/speakup/main.c | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index c2f70ef..396a57a 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -1216,13 +1216,17 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer) u_char ch, version, num_keys; version = *cp++; - if (version != KEY_MAP_VER) - return -1; + if (version != KEY_MAP_VER) { + pr_debug("Version mismatch %d\n", -1); + return -EINVAL; + } num_keys = *cp; states = (int)cp[1]; key_data_len = (states + 1) * (num_keys + 1); - if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) - return -2; + if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) { + pr_debug("Data len and Shift table too big %d\n", -2); + return -EINVAL; + } memset(k_buffer, 0, SHIFT_TBL_SIZE); memset(spk_our_keys, 0, sizeof(spk_our_keys)); spk_shift_table = k_buffer; @@ -1233,14 +1237,18 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer) cp1 += 2; /* now pointing at shift states */ for (i = 1; i <= states; i++) { ch = *cp1++; - if (ch >= SHIFT_TBL_SIZE) - return -3; + if (ch >= SHIFT_TBL_SIZE) { + pr_debug("Key table size overflow %d\n", -3); + return -EINVAL; + } spk_shift_table[ch] = i; } keymap_flags = *cp1++; while ((ch = *cp1)) { - if (ch >= MAX_KEY) - return -4; + if (ch >= MAX_KEY) { + pr_debug("Max Key overflow %d\n", -4); + return -EINVAL; + } spk_our_keys[ch] = cp1; cp1 += states + 1; } -- 2.10.2
[PATCH] return same error value from spk_set_key_info
This patch makes spk_set_key_info return -EINVAL in case of failure instead of returning 4 different values for the type of error that occurred. Retain the previous error values as debug messages instead. Signed-off-by: Pranay Kr. Srivastava --- drivers/staging/speakup/main.c | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index c2f70ef..396a57a 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -1216,13 +1216,17 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer) u_char ch, version, num_keys; version = *cp++; - if (version != KEY_MAP_VER) - return -1; + if (version != KEY_MAP_VER) { + pr_debug("Version mismatch %d\n", -1); + return -EINVAL; + } num_keys = *cp; states = (int)cp[1]; key_data_len = (states + 1) * (num_keys + 1); - if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) - return -2; + if (key_data_len + SHIFT_TBL_SIZE + 4 >= sizeof(spk_key_buf)) { + pr_debug("Data len and Shift table too big %d\n", -2); + return -EINVAL; + } memset(k_buffer, 0, SHIFT_TBL_SIZE); memset(spk_our_keys, 0, sizeof(spk_our_keys)); spk_shift_table = k_buffer; @@ -1233,14 +1237,18 @@ int spk_set_key_info(const u_char *key_info, u_char *k_buffer) cp1 += 2; /* now pointing at shift states */ for (i = 1; i <= states; i++) { ch = *cp1++; - if (ch >= SHIFT_TBL_SIZE) - return -3; + if (ch >= SHIFT_TBL_SIZE) { + pr_debug("Key table size overflow %d\n", -3); + return -EINVAL; + } spk_shift_table[ch] = i; } keymap_flags = *cp1++; while ((ch = *cp1)) { - if (ch >= MAX_KEY) - return -4; + if (ch >= MAX_KEY) { + pr_debug("Max Key overflow %d\n", -4); + return -EINVAL; + } spk_our_keys[ch] = cp1; cp1 += states + 1; } -- 2.10.2
[PATCH v5 3/4] make nbd device wait for its users
When a timeout occurs or a recv fails, then instead of abruptly killing nbd block device wait for its users to finish. This is more required when filesystem(s) like ext2 or ext3 don't expect their buffer heads to disappear while the filesystem is mounted. Each open is now refcounted with the device being released for re-use only when there are no "other users". A timedout or a disconnected device, if in use, can't be used until it has been resetted. The reset happens when all tasks having this bdev open closes this bdev. Behavioral Change: 1) NBD_DO_IT will not wait for the device to be reset. Hence the nbd-client "may exit" while some other process is using this device without actually doing the reset on this device , hence thus making it unusable until all such user space processes have stopped using this device. 2) There's a window where the nbd-client will not be able to change / issue ioctls to the nbd device. This is when there's been a disconnect issued or a timeout has occured, however there are "other" user processes which currently have this nbd device opened. Signed-off-by: Pranay Kr Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 37 - 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 4919760..fe36280 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -74,6 +74,7 @@ struct nbd_device { *This is specifically for calling sock_shutdown, for now. */ struct work_struct ws_shutdown; + atomic_t users; /* Users that opened the block device */ }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -699,6 +700,9 @@ static void nbd_dev_dbg_close(struct nbd_device *nbd); static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, unsigned int cmd, unsigned long arg) { + if (nbd->disconnect || nbd->timedout) + return -EBUSY; + switch (cmd) { case NBD_DISCONNECT: { struct request sreq; @@ -728,7 +732,6 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, nbd_clear_que(nbd); BUG_ON(!list_empty(>queue_head)); BUG_ON(!list_empty(>waiting_queue)); - kill_bdev(bdev); return 0; case NBD_SET_SOCK: { @@ -804,16 +807,12 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, mutex_lock(>tx_lock); nbd->task_recv = NULL; nbd_clear_que(nbd); - kill_bdev(bdev); - nbd_bdev_reset(bdev); if (nbd->disconnect) /* user requested, ignore socket errors */ error = 0; if (nbd->timedout) error = -ETIMEDOUT; - nbd_reset(nbd); - return error; } @@ -852,10 +851,38 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t mode, return error; } +static int nbd_open(struct block_device *bdev, fmode_t mode) +{ + struct nbd_device *nbd = bdev->bd_disk->private_data; + + atomic_inc(>users); + + return 0; +} + +static void nbd_release(struct gendisk *disk, fmode_t mode) +{ + struct nbd_device *nbd = disk->private_data; + struct block_device *bdev = bdget (part_devt( + dev_to_part(nbd_to_dev(nbd; + + WARN_ON(!bdev); + if (atomic_dec_and_test(>users)) { + if (bdev) { + nbd_bdev_reset(bdev); + kill_bdev(bdev); + bdput(bdev); + } + nbd_reset(nbd); + } +} + static const struct block_device_operations nbd_fops = { .owner =THIS_MODULE, .ioctl =nbd_ioctl, .compat_ioctl = nbd_ioctl, + .open = nbd_open, + .release = nbd_release, }; static void nbd_ws_func_shutdown(struct work_struct *ws_nbd) -- 2.6.2
[PATCH v5 3/4] make nbd device wait for its users
When a timeout occurs or a recv fails, then instead of abruptly killing nbd block device wait for its users to finish. This is more required when filesystem(s) like ext2 or ext3 don't expect their buffer heads to disappear while the filesystem is mounted. Each open is now refcounted with the device being released for re-use only when there are no "other users". A timedout or a disconnected device, if in use, can't be used until it has been resetted. The reset happens when all tasks having this bdev open closes this bdev. Behavioral Change: 1) NBD_DO_IT will not wait for the device to be reset. Hence the nbd-client "may exit" while some other process is using this device without actually doing the reset on this device , hence thus making it unusable until all such user space processes have stopped using this device. 2) There's a window where the nbd-client will not be able to change / issue ioctls to the nbd device. This is when there's been a disconnect issued or a timeout has occured, however there are "other" user processes which currently have this nbd device opened. Signed-off-by: Pranay Kr Srivastava --- drivers/block/nbd.c | 37 - 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 4919760..fe36280 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -74,6 +74,7 @@ struct nbd_device { *This is specifically for calling sock_shutdown, for now. */ struct work_struct ws_shutdown; + atomic_t users; /* Users that opened the block device */ }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -699,6 +700,9 @@ static void nbd_dev_dbg_close(struct nbd_device *nbd); static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, unsigned int cmd, unsigned long arg) { + if (nbd->disconnect || nbd->timedout) + return -EBUSY; + switch (cmd) { case NBD_DISCONNECT: { struct request sreq; @@ -728,7 +732,6 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, nbd_clear_que(nbd); BUG_ON(!list_empty(>queue_head)); BUG_ON(!list_empty(>waiting_queue)); - kill_bdev(bdev); return 0; case NBD_SET_SOCK: { @@ -804,16 +807,12 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, mutex_lock(>tx_lock); nbd->task_recv = NULL; nbd_clear_que(nbd); - kill_bdev(bdev); - nbd_bdev_reset(bdev); if (nbd->disconnect) /* user requested, ignore socket errors */ error = 0; if (nbd->timedout) error = -ETIMEDOUT; - nbd_reset(nbd); - return error; } @@ -852,10 +851,38 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t mode, return error; } +static int nbd_open(struct block_device *bdev, fmode_t mode) +{ + struct nbd_device *nbd = bdev->bd_disk->private_data; + + atomic_inc(>users); + + return 0; +} + +static void nbd_release(struct gendisk *disk, fmode_t mode) +{ + struct nbd_device *nbd = disk->private_data; + struct block_device *bdev = bdget (part_devt( + dev_to_part(nbd_to_dev(nbd; + + WARN_ON(!bdev); + if (atomic_dec_and_test(>users)) { + if (bdev) { + nbd_bdev_reset(bdev); + kill_bdev(bdev); + bdput(bdev); + } + nbd_reset(nbd); + } +} + static const struct block_device_operations nbd_fops = { .owner =THIS_MODULE, .ioctl =nbd_ioctl, .compat_ioctl = nbd_ioctl, + .open = nbd_open, + .release = nbd_release, }; static void nbd_ws_func_shutdown(struct work_struct *ws_nbd) -- 2.6.2
[PATCH v5 2/4] nbd: fix might_sleep warning on socket shutdown.
From: "Pranay Kr. Srivastava" <pran...@gmail.com> spinlocked ranges should be small and not contain calls into huge subfunctions. Fix my mistake and just get the pointer to the socket instead of doing everything with spinlock held. Reported-by: Mikulas Patocka <miku...@twibright.com> Signed-off-by: Markus Pargmann <m...@pengutronix.de> Changelog: Pranay Kr. Srivastava<pran...@gmail.com>: 1) Use spin_lock instead of irq version for sock_shutdown. 2) Use system work queue to actually trigger the shutdown of socket. This solves the issue when kernel_sendmsg is currently blocked while a timeout occurs. v5) removed unnecessary code as per review of v4). Signed-off-by: Pranay Kr Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 53 +++-- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 766c401..4919760 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -39,6 +39,7 @@ #include #include +#include struct nbd_device { u32 flags; @@ -69,6 +70,10 @@ struct nbd_device { #if IS_ENABLED(CONFIG_DEBUG_FS) struct dentry *dbg_dir; #endif + /* +*This is specifically for calling sock_shutdown, for now. +*/ + struct work_struct ws_shutdown; }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -95,6 +100,11 @@ static int max_part; */ static DEFINE_SPINLOCK(nbd_lock); +/* + * Shutdown function for nbd_dev work struct. + */ +static void nbd_ws_func_shutdown(struct work_struct *); + static inline struct device *nbd_to_dev(struct nbd_device *nbd) { return disk_to_dev(nbd->disk); @@ -172,39 +182,30 @@ static void nbd_end_request(struct nbd_device *nbd, struct request *req) */ static void sock_shutdown(struct nbd_device *nbd) { - spin_lock_irq(>sock_lock); - - if (!nbd->sock) { - spin_unlock_irq(>sock_lock); - return; - } + struct socket *sock; - dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - sockfd_put(nbd->sock); + spin_lock(>sock_lock); + sock = nbd->sock; nbd->sock = NULL; - spin_unlock_irq(>sock_lock); + spin_unlock(>sock_lock); + + if (!sock) + return; del_timer(>timeout_timer); + dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); + kernel_sock_shutdown(sock, SHUT_RDWR); + sockfd_put(sock); } static void nbd_xmit_timeout(unsigned long arg) { struct nbd_device *nbd = (struct nbd_device *)arg; - unsigned long flags; if (list_empty(>queue_head)) return; - - spin_lock_irqsave(>sock_lock, flags); - nbd->timedout = true; - - if (nbd->sock) - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - - spin_unlock_irqrestore(>sock_lock, flags); - + schedule_work(>ws_shutdown); dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down connection\n"); } @@ -663,6 +664,7 @@ static void nbd_reset(struct nbd_device *nbd) set_capacity(nbd->disk, 0); nbd->flags = 0; nbd->xmit_timeout = 0; + INIT_WORK(>ws_shutdown, nbd_ws_func_shutdown); queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue); del_timer_sync(>timeout_timer); } @@ -798,10 +800,9 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, nbd_dev_dbg_close(nbd); kthread_stop(thread); + sock_shutdown(nbd); mutex_lock(>tx_lock); nbd->task_recv = NULL; - - sock_shutdown(nbd); nbd_clear_que(nbd); kill_bdev(bdev); nbd_bdev_reset(bdev); @@ -857,6 +858,14 @@ static const struct block_device_operations nbd_fops = { .compat_ioctl = nbd_ioctl, }; +static void nbd_ws_func_shutdown(struct work_struct *ws_nbd) +{ + struct nbd_device *nbd_dev = container_of(ws_nbd, struct nbd_device, + ws_shutdown); + + sock_shutdown(nbd_dev); +} + #if IS_ENABLED(CONFIG_DEBUG_FS) static int nbd_dbg_tasks_show(struct seq_file *s, void *unused) -- 2.6.2
[PATCH v5 2/4] nbd: fix might_sleep warning on socket shutdown.
From: "Pranay Kr. Srivastava" spinlocked ranges should be small and not contain calls into huge subfunctions. Fix my mistake and just get the pointer to the socket instead of doing everything with spinlock held. Reported-by: Mikulas Patocka Signed-off-by: Markus Pargmann Changelog: Pranay Kr. Srivastava: 1) Use spin_lock instead of irq version for sock_shutdown. 2) Use system work queue to actually trigger the shutdown of socket. This solves the issue when kernel_sendmsg is currently blocked while a timeout occurs. v5) removed unnecessary code as per review of v4). Signed-off-by: Pranay Kr Srivastava --- drivers/block/nbd.c | 53 +++-- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 766c401..4919760 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -39,6 +39,7 @@ #include #include +#include struct nbd_device { u32 flags; @@ -69,6 +70,10 @@ struct nbd_device { #if IS_ENABLED(CONFIG_DEBUG_FS) struct dentry *dbg_dir; #endif + /* +*This is specifically for calling sock_shutdown, for now. +*/ + struct work_struct ws_shutdown; }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -95,6 +100,11 @@ static int max_part; */ static DEFINE_SPINLOCK(nbd_lock); +/* + * Shutdown function for nbd_dev work struct. + */ +static void nbd_ws_func_shutdown(struct work_struct *); + static inline struct device *nbd_to_dev(struct nbd_device *nbd) { return disk_to_dev(nbd->disk); @@ -172,39 +182,30 @@ static void nbd_end_request(struct nbd_device *nbd, struct request *req) */ static void sock_shutdown(struct nbd_device *nbd) { - spin_lock_irq(>sock_lock); - - if (!nbd->sock) { - spin_unlock_irq(>sock_lock); - return; - } + struct socket *sock; - dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - sockfd_put(nbd->sock); + spin_lock(>sock_lock); + sock = nbd->sock; nbd->sock = NULL; - spin_unlock_irq(>sock_lock); + spin_unlock(>sock_lock); + + if (!sock) + return; del_timer(>timeout_timer); + dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); + kernel_sock_shutdown(sock, SHUT_RDWR); + sockfd_put(sock); } static void nbd_xmit_timeout(unsigned long arg) { struct nbd_device *nbd = (struct nbd_device *)arg; - unsigned long flags; if (list_empty(>queue_head)) return; - - spin_lock_irqsave(>sock_lock, flags); - nbd->timedout = true; - - if (nbd->sock) - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - - spin_unlock_irqrestore(>sock_lock, flags); - + schedule_work(>ws_shutdown); dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down connection\n"); } @@ -663,6 +664,7 @@ static void nbd_reset(struct nbd_device *nbd) set_capacity(nbd->disk, 0); nbd->flags = 0; nbd->xmit_timeout = 0; + INIT_WORK(>ws_shutdown, nbd_ws_func_shutdown); queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue); del_timer_sync(>timeout_timer); } @@ -798,10 +800,9 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, nbd_dev_dbg_close(nbd); kthread_stop(thread); + sock_shutdown(nbd); mutex_lock(>tx_lock); nbd->task_recv = NULL; - - sock_shutdown(nbd); nbd_clear_que(nbd); kill_bdev(bdev); nbd_bdev_reset(bdev); @@ -857,6 +858,14 @@ static const struct block_device_operations nbd_fops = { .compat_ioctl = nbd_ioctl, }; +static void nbd_ws_func_shutdown(struct work_struct *ws_nbd) +{ + struct nbd_device *nbd_dev = container_of(ws_nbd, struct nbd_device, + ws_shutdown); + + sock_shutdown(nbd_dev); +} + #if IS_ENABLED(CONFIG_DEBUG_FS) static int nbd_dbg_tasks_show(struct seq_file *s, void *unused) -- 2.6.2
[no subject]
Hi Markus, Can you take a look at this. Let me know if this looks ok, I'll resend the whole series again. Regards,
[no subject]
Hi Markus, Can you take a look at this. Let me know if this looks ok, I'll resend the whole series again. Regards,
[PATCH 0/1]ext4: Fix for WARN_ON_ONCE when marking buffer dirty
1) Fix WARN_ON_ONCE when marking buffer dirty it's possible that a writeback for the super block buffer head is triggered after setting the buffer uptodate on a buffer write_io_error. If however there's an error while writing the buffer head then the buffer is cleared from being uptodate. If the buffer uptodate is not set while calling mark_buffer_dirty, it throws a WARN_ON_ONCE. This patch fixes it by locking the buffer while marking the buffer uptodate and then marking it dirty while holding the buffer head lock. Pranay Kr. Srivastava (5): Fix WARN_ON_ONCE when marking buffer dirty fs/ext4/super.c | 30 +- 1 files changed, 16 insertions(+), 14 deletions(-) -- 1.9.1
[PATCH 0/1]ext4: Fix for WARN_ON_ONCE when marking buffer dirty
1) Fix WARN_ON_ONCE when marking buffer dirty it's possible that a writeback for the super block buffer head is triggered after setting the buffer uptodate on a buffer write_io_error. If however there's an error while writing the buffer head then the buffer is cleared from being uptodate. If the buffer uptodate is not set while calling mark_buffer_dirty, it throws a WARN_ON_ONCE. This patch fixes it by locking the buffer while marking the buffer uptodate and then marking it dirty while holding the buffer head lock. Pranay Kr. Srivastava (5): Fix WARN_ON_ONCE when marking buffer dirty fs/ext4/super.c | 30 +- 1 files changed, 16 insertions(+), 14 deletions(-) -- 1.9.1
[PATCH 1/1]ext4: Fix WARN_ON_ONCE when marking buffer dirty
Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- fs/ext4/super.c | 30 -- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 3822a5a..8f10715 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -4327,20 +4327,6 @@ static int ext4_commit_super(struct super_block *sb, int sync) if (!sbh || block_device_ejected(sb)) return error; - if (buffer_write_io_error(sbh)) { - /* -* Oh, dear. A previous attempt to write the -* superblock failed. This could happen because the -* USB device was yanked out. Or it could happen to -* be a transient write error and maybe the block will -* be remapped. Nothing we can do but to retry the -* write and hope for the best. -*/ - ext4_msg(sb, KERN_ERR, "previous I/O error to " - "superblock detected"); - clear_buffer_write_io_error(sbh); - set_buffer_uptodate(sbh); - } /* * If the file system is mounted read-only, don't update the * superblock write time. This avoids updating the superblock @@ -4371,7 +4357,23 @@ static int ext4_commit_super(struct super_block *sb, int sync) _SB(sb)->s_freeinodes_counter)); BUFFER_TRACE(sbh, "marking dirty"); ext4_superblock_csum_set(sb); + lock_buffer(sbh); + if (buffer_write_io_error(sbh)) { + /* +* Oh, dear. A previous attempt to write the +* superblock failed. This could happen because the +* USB device was yanked out. Or it could happen to +* be a transient write error and maybe the block will +* be remapped. Nothing we can do but to retry the +* write and hope for the best. +*/ + ext4_msg(sb, KERN_ERR, "previous I/O error to " + "superblock detected"); + clear_buffer_write_io_error(sbh); + set_buffer_uptodate(sbh); + } mark_buffer_dirty(sbh); + unlock_buffer(sbh); if (sync) { error = __sync_dirty_buffer(sbh, test_opt(sb, BARRIER) ? WRITE_FUA : WRITE_SYNC); -- 1.9.1
[PATCH 1/1]ext4: Fix WARN_ON_ONCE when marking buffer dirty
Signed-off-by: Pranay Kr. Srivastava --- fs/ext4/super.c | 30 -- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 3822a5a..8f10715 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -4327,20 +4327,6 @@ static int ext4_commit_super(struct super_block *sb, int sync) if (!sbh || block_device_ejected(sb)) return error; - if (buffer_write_io_error(sbh)) { - /* -* Oh, dear. A previous attempt to write the -* superblock failed. This could happen because the -* USB device was yanked out. Or it could happen to -* be a transient write error and maybe the block will -* be remapped. Nothing we can do but to retry the -* write and hope for the best. -*/ - ext4_msg(sb, KERN_ERR, "previous I/O error to " - "superblock detected"); - clear_buffer_write_io_error(sbh); - set_buffer_uptodate(sbh); - } /* * If the file system is mounted read-only, don't update the * superblock write time. This avoids updating the superblock @@ -4371,7 +4357,23 @@ static int ext4_commit_super(struct super_block *sb, int sync) _SB(sb)->s_freeinodes_counter)); BUFFER_TRACE(sbh, "marking dirty"); ext4_superblock_csum_set(sb); + lock_buffer(sbh); + if (buffer_write_io_error(sbh)) { + /* +* Oh, dear. A previous attempt to write the +* superblock failed. This could happen because the +* USB device was yanked out. Or it could happen to +* be a transient write error and maybe the block will +* be remapped. Nothing we can do but to retry the +* write and hope for the best. +*/ + ext4_msg(sb, KERN_ERR, "previous I/O error to " + "superblock detected"); + clear_buffer_write_io_error(sbh); + set_buffer_uptodate(sbh); + } mark_buffer_dirty(sbh); + unlock_buffer(sbh); if (sync) { error = __sync_dirty_buffer(sbh, test_opt(sb, BARRIER) ? WRITE_FUA : WRITE_SYNC); -- 1.9.1
[PATCH v4 3/5]nbd: make nbd device wait for its users
When a timeout occurs or a recv fails, then instead of abruplty killing nbd block device wait for its users to finish. This is more required when filesystem(s) like ext2 or ext3 don't expect their buffer heads to disappear while the filesystem is mounted. Each open of a nbd device is refcounted, while the userland program [nbd-client] doing the NBD_DO_IT ioctl would now wait for any other users of this device before invalidating the nbd device. A timedout or a disconnected device, if in use, can't be used until it has been resetted. The reset happens when all tasks having this bdev open closes this bdev. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 106 ++-- 1 file changed, 87 insertions(+), 19 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index e362d44..fb56dd2 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -72,6 +72,8 @@ struct nbd_device { #endif /* This is specifically for calling sock_shutdown, for now. */ struct work_struct ws_shutdown; + struct kref users; + struct completion user_completion; }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -99,6 +101,8 @@ static int max_part; static DEFINE_SPINLOCK(nbd_lock); static void nbd_ws_func_shutdown(struct work_struct *); +static void nbd_kref_release(struct kref *); +static int nbd_size_clear(struct nbd_device *, struct block_device *); static inline struct device *nbd_to_dev(struct nbd_device *nbd) { @@ -145,11 +149,9 @@ static int nbd_size_set(struct nbd_device *nbd, struct block_device *bdev, int blocksize, int nr_blocks) { int ret; - ret = set_blocksize(bdev, blocksize); if (ret) return ret; - nbd->blksize = blocksize; nbd->bytesize = (loff_t)blocksize * (loff_t)nr_blocks; @@ -197,6 +199,9 @@ static void nbd_xmit_timeout(unsigned long arg) { struct nbd_device *nbd = (struct nbd_device *)arg; + if (nbd->timedout) + return; + if (list_empty(>queue_head)) return; @@ -472,8 +477,6 @@ static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev) nbd_end_request(nbd, req); } - nbd_size_clear(nbd, bdev); - device_remove_file(disk_to_dev(nbd->disk), _attr_pid); nbd->task_recv = NULL; @@ -650,12 +653,13 @@ static int nbd_set_socket(struct nbd_device *nbd, struct socket *sock) int ret = 0; spin_lock(>sock_lock); - if (nbd->sock) + + if (nbd->sock || nbd->timedout) ret = -EBUSY; else nbd->sock = sock; - spin_unlock(>sock_lock); + spin_unlock(>sock_lock); return ret; } @@ -670,6 +674,7 @@ static void nbd_reset(struct nbd_device *nbd) nbd->flags = 0; nbd->xmit_timeout = 0; INIT_WORK(>ws_shutdown, nbd_ws_func_shutdown); + init_completion(>user_completion); queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue); del_timer_sync(>timeout_timer); } @@ -704,6 +709,9 @@ static void nbd_dev_dbg_close(struct nbd_device *nbd); static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, unsigned int cmd, unsigned long arg) { + if (nbd->timedout || nbd->disconnect) + return -EBUSY; + switch (cmd) { case NBD_DISCONNECT: { struct request sreq; @@ -733,7 +741,6 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, nbd_clear_que(nbd); BUG_ON(!list_empty(>queue_head)); BUG_ON(!list_empty(>waiting_queue)); - kill_bdev(bdev); return 0; case NBD_SET_SOCK: { @@ -752,7 +759,6 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, case NBD_SET_BLKSIZE: { loff_t bsize = div_s64(nbd->bytesize, arg); - return nbd_size_set(nbd, bdev, arg, bsize); } @@ -804,22 +810,29 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, error = nbd_thread_recv(nbd, bdev); nbd_dev_dbg_close(nbd); kthread_stop(thread); - sock_shutdown(nbd); - - mutex_lock(>tx_lock); - nbd->task_recv = NULL; - nbd_clear_que(nbd); - kill_bdev(bdev); - nbd_bdev_reset(bdev); + sock_shutdown(nbd); if (nbd->disconnect) /* user requested, ignore socket errors */ error = 0; if (nbd->timedout) error = -ETIMEDOUT; - nbd_reset(nbd); + mutex_lock(>tx_lo
[PATCH v4 3/5]nbd: make nbd device wait for its users
When a timeout occurs or a recv fails, then instead of abruplty killing nbd block device wait for its users to finish. This is more required when filesystem(s) like ext2 or ext3 don't expect their buffer heads to disappear while the filesystem is mounted. Each open of a nbd device is refcounted, while the userland program [nbd-client] doing the NBD_DO_IT ioctl would now wait for any other users of this device before invalidating the nbd device. A timedout or a disconnected device, if in use, can't be used until it has been resetted. The reset happens when all tasks having this bdev open closes this bdev. Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 106 ++-- 1 file changed, 87 insertions(+), 19 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index e362d44..fb56dd2 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -72,6 +72,8 @@ struct nbd_device { #endif /* This is specifically for calling sock_shutdown, for now. */ struct work_struct ws_shutdown; + struct kref users; + struct completion user_completion; }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -99,6 +101,8 @@ static int max_part; static DEFINE_SPINLOCK(nbd_lock); static void nbd_ws_func_shutdown(struct work_struct *); +static void nbd_kref_release(struct kref *); +static int nbd_size_clear(struct nbd_device *, struct block_device *); static inline struct device *nbd_to_dev(struct nbd_device *nbd) { @@ -145,11 +149,9 @@ static int nbd_size_set(struct nbd_device *nbd, struct block_device *bdev, int blocksize, int nr_blocks) { int ret; - ret = set_blocksize(bdev, blocksize); if (ret) return ret; - nbd->blksize = blocksize; nbd->bytesize = (loff_t)blocksize * (loff_t)nr_blocks; @@ -197,6 +199,9 @@ static void nbd_xmit_timeout(unsigned long arg) { struct nbd_device *nbd = (struct nbd_device *)arg; + if (nbd->timedout) + return; + if (list_empty(>queue_head)) return; @@ -472,8 +477,6 @@ static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev) nbd_end_request(nbd, req); } - nbd_size_clear(nbd, bdev); - device_remove_file(disk_to_dev(nbd->disk), _attr_pid); nbd->task_recv = NULL; @@ -650,12 +653,13 @@ static int nbd_set_socket(struct nbd_device *nbd, struct socket *sock) int ret = 0; spin_lock(>sock_lock); - if (nbd->sock) + + if (nbd->sock || nbd->timedout) ret = -EBUSY; else nbd->sock = sock; - spin_unlock(>sock_lock); + spin_unlock(>sock_lock); return ret; } @@ -670,6 +674,7 @@ static void nbd_reset(struct nbd_device *nbd) nbd->flags = 0; nbd->xmit_timeout = 0; INIT_WORK(>ws_shutdown, nbd_ws_func_shutdown); + init_completion(>user_completion); queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue); del_timer_sync(>timeout_timer); } @@ -704,6 +709,9 @@ static void nbd_dev_dbg_close(struct nbd_device *nbd); static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, unsigned int cmd, unsigned long arg) { + if (nbd->timedout || nbd->disconnect) + return -EBUSY; + switch (cmd) { case NBD_DISCONNECT: { struct request sreq; @@ -733,7 +741,6 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, nbd_clear_que(nbd); BUG_ON(!list_empty(>queue_head)); BUG_ON(!list_empty(>waiting_queue)); - kill_bdev(bdev); return 0; case NBD_SET_SOCK: { @@ -752,7 +759,6 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, case NBD_SET_BLKSIZE: { loff_t bsize = div_s64(nbd->bytesize, arg); - return nbd_size_set(nbd, bdev, arg, bsize); } @@ -804,22 +810,29 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, error = nbd_thread_recv(nbd, bdev); nbd_dev_dbg_close(nbd); kthread_stop(thread); - sock_shutdown(nbd); - - mutex_lock(>tx_lock); - nbd->task_recv = NULL; - nbd_clear_que(nbd); - kill_bdev(bdev); - nbd_bdev_reset(bdev); + sock_shutdown(nbd); if (nbd->disconnect) /* user requested, ignore socket errors */ error = 0; if (nbd->timedout) error = -ETIMEDOUT; - nbd_reset(nbd); + mutex_lock(>tx_lock); + nbd_clear_que(nbd); +
[PATCH v4 2/5]nbd: fix might_sleep warning on socket shutdown
spinlocked ranges should be small and not contain calls into huge subfunctions. Fix my mistake and just get the pointer to the socket instead of doing everything with spinlock held. Reported-by: Mikulas Patocka <miku...@twibright.com> Signed-off-by: Markus Pargmann <m...@pengutronix.de> Changelog: Pranay Kr. Srivastava<pran...@gmail.com>: 1) Use spin_lock instead of irq version for sock_shutdown. 2) Use system work queue to actually trigger the shutdown of socket. This solves the issue when kernel_sendmsg is currently blocked while a timeout occurs. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 57 + 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 766c401..e362d44 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -39,6 +39,7 @@ #include #include +#include struct nbd_device { u32 flags; @@ -69,6 +70,8 @@ struct nbd_device { #if IS_ENABLED(CONFIG_DEBUG_FS) struct dentry *dbg_dir; #endif + /* This is specifically for calling sock_shutdown, for now. */ + struct work_struct ws_shutdown; }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -95,6 +98,8 @@ static int max_part; */ static DEFINE_SPINLOCK(nbd_lock); +static void nbd_ws_func_shutdown(struct work_struct *); + static inline struct device *nbd_to_dev(struct nbd_device *nbd) { return disk_to_dev(nbd->disk); @@ -172,39 +177,36 @@ static void nbd_end_request(struct nbd_device *nbd, struct request *req) */ static void sock_shutdown(struct nbd_device *nbd) { - spin_lock_irq(>sock_lock); - - if (!nbd->sock) { - spin_unlock_irq(>sock_lock); - return; - } + struct socket *sock; - dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - sockfd_put(nbd->sock); + spin_lock(>sock_lock); + sock = nbd->sock; nbd->sock = NULL; - spin_unlock_irq(>sock_lock); + spin_unlock(>sock_lock); + + if (!sock) + return; del_timer(>timeout_timer); + dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); + kernel_sock_shutdown(sock, SHUT_RDWR); + sockfd_put(sock); } static void nbd_xmit_timeout(unsigned long arg) { struct nbd_device *nbd = (struct nbd_device *)arg; - unsigned long flags; if (list_empty(>queue_head)) return; - spin_lock_irqsave(>sock_lock, flags); - nbd->timedout = true; - - if (nbd->sock) - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - - spin_unlock_irqrestore(>sock_lock, flags); - + /* +* Make sure sender thread sees nbd->timedout. +*/ + smp_wmb(); + schedule_work(>ws_shutdown); + wake_up(>waiting_wq); dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down connection\n"); } @@ -588,7 +590,11 @@ static int nbd_thread_send(void *data) spin_unlock_irq(>queue_lock); /* handle request */ - nbd_handle_req(nbd, req); + if (nbd->timedout) { + req->errors++; + nbd_end_request(nbd, req); + } else + nbd_handle_req(nbd, req); } nbd->task_send = NULL; @@ -663,6 +669,7 @@ static void nbd_reset(struct nbd_device *nbd) set_capacity(nbd->disk, 0); nbd->flags = 0; nbd->xmit_timeout = 0; + INIT_WORK(>ws_shutdown, nbd_ws_func_shutdown); queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue); del_timer_sync(>timeout_timer); } @@ -797,11 +804,11 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, error = nbd_thread_recv(nbd, bdev); nbd_dev_dbg_close(nbd); kthread_stop(thread); + sock_shutdown(nbd); mutex_lock(>tx_lock); nbd->task_recv = NULL; - sock_shutdown(nbd); nbd_clear_que(nbd); kill_bdev(bdev); nbd_bdev_reset(bdev); @@ -857,6 +864,14 @@ static const struct block_device_operations nbd_fops = { .compat_ioctl = nbd_ioctl, }; +static void nbd_ws_func_shutdown(struct work_struct *ws_nbd) +{ + struct nbd_device *nbd_dev = container_of(ws_nbd, struct nbd_device, + ws_shutdown); + + sock_shutdown(nbd_dev); +} + #if IS_ENABLED(CONFIG_DEBUG_FS) static int nbd_dbg_tasks_show(struct seq_file *s, void *unused) -- 1.9.1
[PATCH v4 2/5]nbd: fix might_sleep warning on socket shutdown
spinlocked ranges should be small and not contain calls into huge subfunctions. Fix my mistake and just get the pointer to the socket instead of doing everything with spinlock held. Reported-by: Mikulas Patocka Signed-off-by: Markus Pargmann Changelog: Pranay Kr. Srivastava: 1) Use spin_lock instead of irq version for sock_shutdown. 2) Use system work queue to actually trigger the shutdown of socket. This solves the issue when kernel_sendmsg is currently blocked while a timeout occurs. Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 57 + 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 766c401..e362d44 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -39,6 +39,7 @@ #include #include +#include struct nbd_device { u32 flags; @@ -69,6 +70,8 @@ struct nbd_device { #if IS_ENABLED(CONFIG_DEBUG_FS) struct dentry *dbg_dir; #endif + /* This is specifically for calling sock_shutdown, for now. */ + struct work_struct ws_shutdown; }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -95,6 +98,8 @@ static int max_part; */ static DEFINE_SPINLOCK(nbd_lock); +static void nbd_ws_func_shutdown(struct work_struct *); + static inline struct device *nbd_to_dev(struct nbd_device *nbd) { return disk_to_dev(nbd->disk); @@ -172,39 +177,36 @@ static void nbd_end_request(struct nbd_device *nbd, struct request *req) */ static void sock_shutdown(struct nbd_device *nbd) { - spin_lock_irq(>sock_lock); - - if (!nbd->sock) { - spin_unlock_irq(>sock_lock); - return; - } + struct socket *sock; - dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - sockfd_put(nbd->sock); + spin_lock(>sock_lock); + sock = nbd->sock; nbd->sock = NULL; - spin_unlock_irq(>sock_lock); + spin_unlock(>sock_lock); + + if (!sock) + return; del_timer(>timeout_timer); + dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); + kernel_sock_shutdown(sock, SHUT_RDWR); + sockfd_put(sock); } static void nbd_xmit_timeout(unsigned long arg) { struct nbd_device *nbd = (struct nbd_device *)arg; - unsigned long flags; if (list_empty(>queue_head)) return; - spin_lock_irqsave(>sock_lock, flags); - nbd->timedout = true; - - if (nbd->sock) - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - - spin_unlock_irqrestore(>sock_lock, flags); - + /* +* Make sure sender thread sees nbd->timedout. +*/ + smp_wmb(); + schedule_work(>ws_shutdown); + wake_up(>waiting_wq); dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down connection\n"); } @@ -588,7 +590,11 @@ static int nbd_thread_send(void *data) spin_unlock_irq(>queue_lock); /* handle request */ - nbd_handle_req(nbd, req); + if (nbd->timedout) { + req->errors++; + nbd_end_request(nbd, req); + } else + nbd_handle_req(nbd, req); } nbd->task_send = NULL; @@ -663,6 +669,7 @@ static void nbd_reset(struct nbd_device *nbd) set_capacity(nbd->disk, 0); nbd->flags = 0; nbd->xmit_timeout = 0; + INIT_WORK(>ws_shutdown, nbd_ws_func_shutdown); queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue); del_timer_sync(>timeout_timer); } @@ -797,11 +804,11 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, error = nbd_thread_recv(nbd, bdev); nbd_dev_dbg_close(nbd); kthread_stop(thread); + sock_shutdown(nbd); mutex_lock(>tx_lock); nbd->task_recv = NULL; - sock_shutdown(nbd); nbd_clear_que(nbd); kill_bdev(bdev); nbd_bdev_reset(bdev); @@ -857,6 +864,14 @@ static const struct block_device_operations nbd_fops = { .compat_ioctl = nbd_ioctl, }; +static void nbd_ws_func_shutdown(struct work_struct *ws_nbd) +{ + struct nbd_device *nbd_dev = container_of(ws_nbd, struct nbd_device, + ws_shutdown); + + sock_shutdown(nbd_dev); +} + #if IS_ENABLED(CONFIG_DEBUG_FS) static int nbd_dbg_tasks_show(struct seq_file *s, void *unused) -- 1.9.1
[PATCH v4 4/5]nbd: use i_size_write to assign nbd device size
Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index fb56dd2..7126878 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -128,7 +128,7 @@ static const char *nbdcmd_to_ascii(int cmd) static int nbd_size_clear(struct nbd_device *nbd, struct block_device *bdev) { - bdev->bd_inode->i_size = 0; + i_size_write(bdev->bd_inode, 0); set_capacity(nbd->disk, 0); kobject_uevent(_to_dev(nbd)->kobj, KOBJ_CHANGE); @@ -140,7 +140,7 @@ static void nbd_size_update(struct nbd_device *nbd, struct block_device *bdev) if (!nbd_is_connected(nbd)) return; - bdev->bd_inode->i_size = nbd->bytesize; + i_size_write(bdev->bd_inode, nbd->bytesize); set_capacity(nbd->disk, nbd->bytesize >> 9); kobject_uevent(_to_dev(nbd)->kobj, KOBJ_CHANGE); } @@ -682,7 +682,7 @@ static void nbd_reset(struct nbd_device *nbd) static void nbd_bdev_reset(struct block_device *bdev) { set_device_ro(bdev, false); - bdev->bd_inode->i_size = 0; + i_size_write(bdev->bd_inode, 0); if (max_part > 0) { blkdev_reread_part(bdev); bdev->bd_invalidated = 1; -- 1.9.1
[PATCH v4 0/4] nbd: nbd fixes
This patch series fixes the following 1) cleanup nbd_set_socket Simple fixes to nbd_set_socket. 2) fix might_sleep warning on socket shutdown: Fix sock_shutdown to avoid calling kernel_sock_shutdown while holding spin_lock. 3) make nbd device wait for its users. When a timeout or error occurs then nbd driver simply kills the block device. Many filesystem(s) example ext2/ext3 don't expect their buffer heads to disappear like that. Fix this by making nbd device wait for its users. The same work function is used to trigger the kill_bdev as well do a sock_shutdown, depending on either a timeout/error occured or a disconnect was issued. Also avoid scheduling the work_fn in case a timeout for a request has already occured. 4) use i_size_write to assign nbd device size Instead of directly assigning nbd block device size, use i_size_write for modification. Pranay Kr. Srivastava (4): cleanup nbd_set_socket fix might_sleep warning on socket shutdown make nbd device wait for its users use i_size_write to assign nbd device size drivers/block/nbd.c | 168 ++-- 1 file changed, 123 insertions(+), 45 deletions(-) Changelog: *) Rebased all patches above on git://git.pengutronix.de/git/mpa/linux-nbd.git, commit:7ed71a8704eda7b75fbd0ed73fd0a5b6e469d250 *) Formatting issues, and removed unnecessary code. *) Splitted the patch to wait for users to create a new patch 4) use i_size_write to assign nbd device size -- 1.9.1
[PATCH v4 1/5]nbd: cleanup nbd_set_socket
From: "Pranay Kr. Srivastava" <pranaykumar.srivast...@cognizant.com> This patch 1) uses spin_lock instead of irq version. 2) removes the goto statement in case a socket is already assigned with simple if-else statement. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 15 +-- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 56f7f5d..766c401 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -643,17 +643,12 @@ static int nbd_set_socket(struct nbd_device *nbd, struct socket *sock) { int ret = 0; - spin_lock_irq(>sock_lock); - - if (nbd->sock) { + spin_lock(>sock_lock); + if (nbd->sock) ret = -EBUSY; - goto out; - } - - nbd->sock = sock; - -out: - spin_unlock_irq(>sock_lock); + else + nbd->sock = sock; + spin_unlock(>sock_lock); return ret; } -- 1.9.1
[PATCH v4 4/5]nbd: use i_size_write to assign nbd device size
Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index fb56dd2..7126878 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -128,7 +128,7 @@ static const char *nbdcmd_to_ascii(int cmd) static int nbd_size_clear(struct nbd_device *nbd, struct block_device *bdev) { - bdev->bd_inode->i_size = 0; + i_size_write(bdev->bd_inode, 0); set_capacity(nbd->disk, 0); kobject_uevent(_to_dev(nbd)->kobj, KOBJ_CHANGE); @@ -140,7 +140,7 @@ static void nbd_size_update(struct nbd_device *nbd, struct block_device *bdev) if (!nbd_is_connected(nbd)) return; - bdev->bd_inode->i_size = nbd->bytesize; + i_size_write(bdev->bd_inode, nbd->bytesize); set_capacity(nbd->disk, nbd->bytesize >> 9); kobject_uevent(_to_dev(nbd)->kobj, KOBJ_CHANGE); } @@ -682,7 +682,7 @@ static void nbd_reset(struct nbd_device *nbd) static void nbd_bdev_reset(struct block_device *bdev) { set_device_ro(bdev, false); - bdev->bd_inode->i_size = 0; + i_size_write(bdev->bd_inode, 0); if (max_part > 0) { blkdev_reread_part(bdev); bdev->bd_invalidated = 1; -- 1.9.1
[PATCH v4 0/4] nbd: nbd fixes
This patch series fixes the following 1) cleanup nbd_set_socket Simple fixes to nbd_set_socket. 2) fix might_sleep warning on socket shutdown: Fix sock_shutdown to avoid calling kernel_sock_shutdown while holding spin_lock. 3) make nbd device wait for its users. When a timeout or error occurs then nbd driver simply kills the block device. Many filesystem(s) example ext2/ext3 don't expect their buffer heads to disappear like that. Fix this by making nbd device wait for its users. The same work function is used to trigger the kill_bdev as well do a sock_shutdown, depending on either a timeout/error occured or a disconnect was issued. Also avoid scheduling the work_fn in case a timeout for a request has already occured. 4) use i_size_write to assign nbd device size Instead of directly assigning nbd block device size, use i_size_write for modification. Pranay Kr. Srivastava (4): cleanup nbd_set_socket fix might_sleep warning on socket shutdown make nbd device wait for its users use i_size_write to assign nbd device size drivers/block/nbd.c | 168 ++-- 1 file changed, 123 insertions(+), 45 deletions(-) Changelog: *) Rebased all patches above on git://git.pengutronix.de/git/mpa/linux-nbd.git, commit:7ed71a8704eda7b75fbd0ed73fd0a5b6e469d250 *) Formatting issues, and removed unnecessary code. *) Splitted the patch to wait for users to create a new patch 4) use i_size_write to assign nbd device size -- 1.9.1
[PATCH v4 1/5]nbd: cleanup nbd_set_socket
From: "Pranay Kr. Srivastava" This patch 1) uses spin_lock instead of irq version. 2) removes the goto statement in case a socket is already assigned with simple if-else statement. Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 15 +-- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 56f7f5d..766c401 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -643,17 +643,12 @@ static int nbd_set_socket(struct nbd_device *nbd, struct socket *sock) { int ret = 0; - spin_lock_irq(>sock_lock); - - if (nbd->sock) { + spin_lock(>sock_lock); + if (nbd->sock) ret = -EBUSY; - goto out; - } - - nbd->sock = sock; - -out: - spin_unlock_irq(>sock_lock); + else + nbd->sock = sock; + spin_unlock(>sock_lock); return ret; } -- 1.9.1
[PATCH v3 2/3]nbd: cleanup nbd_set_socket
This patch 1) uses spin_lock instead of irq version. 2) removes the goto statement in case a socket is already assigned with simple if-else statement. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 15 +-- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 586d946..9223b09 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -653,17 +653,12 @@ static int nbd_set_socket(struct nbd_device *nbd, struct socket *sock) { int ret = 0; - spin_lock_irq(>sock_lock); - - if (nbd->sock) { + spin_lock(>sock_lock); + if (nbd->sock) ret = -EBUSY; - goto out; - } - - nbd->sock = sock; - -out: - spin_unlock_irq(>sock_lock); + else + nbd->sock = sock; + spin_unlock(>sock_lock); return ret; } -- 1.9.1
[PATCH v3 2/3]nbd: cleanup nbd_set_socket
This patch 1) uses spin_lock instead of irq version. 2) removes the goto statement in case a socket is already assigned with simple if-else statement. Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 15 +-- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 586d946..9223b09 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -653,17 +653,12 @@ static int nbd_set_socket(struct nbd_device *nbd, struct socket *sock) { int ret = 0; - spin_lock_irq(>sock_lock); - - if (nbd->sock) { + spin_lock(>sock_lock); + if (nbd->sock) ret = -EBUSY; - goto out; - } - - nbd->sock = sock; - -out: - spin_unlock_irq(>sock_lock); + else + nbd->sock = sock; + spin_unlock(>sock_lock); return ret; } -- 1.9.1
[PATCH 3/3]nbd: make nbd device wait for its users
When a timeout occurs or a recv fails, then instead of abruplty killing nbd block device wait for it's users to finish. This is more required when filesystem(s) like ext2 or ext3 don't expect their buffer heads to disappear while the filesystem is mounted. Each open of a nbd device is refcounted, while the userland program [nbd-client] doing the NBD_DO_IT ioctl would now wait for any other users of this device before invalidating the nbd device. A timedout or a disconnected device, if in use, can't be used until it has been resetted. The resetting happens when all tasks having this bdev open closes this bdev. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 124 1 file changed, 96 insertions(+), 28 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 9223b09..0587bbd 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -70,10 +70,13 @@ struct nbd_device { #if IS_ENABLED(CONFIG_DEBUG_FS) struct dentry *dbg_dir; #endif + /* - *This is specifically for calling sock_shutdown, for now. - */ +*This is specifically for calling sock_shutdown, for now. +*/ struct work_struct ws_shutdown; + struct kref users; + struct completion user_completion; }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -104,6 +107,8 @@ static DEFINE_SPINLOCK(nbd_lock); * Shutdown function for nbd_dev work struct. */ static void nbd_ws_func_shutdown(struct work_struct *); +static void nbd_kref_release(struct kref *); +static int nbd_size_clear(struct nbd_device *, struct block_device *); static inline struct device *nbd_to_dev(struct nbd_device *nbd) { @@ -129,7 +134,7 @@ static const char *nbdcmd_to_ascii(int cmd) static int nbd_size_clear(struct nbd_device *nbd, struct block_device *bdev) { - bdev->bd_inode->i_size = 0; + i_size_write(bdev->bd_inode, 0); set_capacity(nbd->disk, 0); kobject_uevent(_to_dev(nbd)->kobj, KOBJ_CHANGE); @@ -141,7 +146,7 @@ static void nbd_size_update(struct nbd_device *nbd, struct block_device *bdev) if (!nbd_is_connected(nbd)) return; - bdev->bd_inode->i_size = nbd->bytesize; + i_size_write(bdev->bd_inode, nbd->bytesize); set_capacity(nbd->disk, nbd->bytesize >> 9); kobject_uevent(_to_dev(nbd)->kobj, KOBJ_CHANGE); } @@ -150,11 +155,9 @@ static int nbd_size_set(struct nbd_device *nbd, struct block_device *bdev, int blocksize, int nr_blocks) { int ret; - ret = set_blocksize(bdev, blocksize); if (ret) return ret; - nbd->blksize = blocksize; nbd->bytesize = (loff_t)blocksize * (loff_t)nr_blocks; @@ -202,14 +205,19 @@ static void nbd_xmit_timeout(unsigned long arg) { struct nbd_device *nbd = (struct nbd_device *)arg; + if (nbd->timedout) + return; + if (list_empty(>queue_head)) return; + nbd->timedout = true; - schedule_work(>ws_shutdown); + /* * Make sure sender thread sees nbd->timedout. */ smp_wmb(); + schedule_work(>ws_shutdown); wake_up(>waiting_wq); dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down connection\n"); } @@ -476,8 +484,6 @@ static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev) nbd_end_request(nbd, req); } - nbd_size_clear(nbd, bdev); - device_remove_file(disk_to_dev(nbd->disk), _attr_pid); nbd->task_recv = NULL; @@ -580,8 +586,8 @@ static int nbd_thread_send(void *data) while (!kthread_should_stop() || !list_empty(>waiting_queue)) { /* wait for something to do */ wait_event_interruptible(nbd->waiting_wq, - kthread_should_stop() || - !list_empty(>waiting_queue)); +kthread_should_stop() || +!list_empty(>waiting_queue)); /* extract request */ if (list_empty(>waiting_queue)) @@ -589,11 +595,11 @@ static int nbd_thread_send(void *data) spin_lock_irq(>queue_lock); req = list_entry(nbd->waiting_queue.next, struct request, - queuelist); +queuelist); list_del_init(>queuelist); spin_unlock_irq(>queue_lock); - nbd_handle_req(nbd, req); + /* handle request */ if (nbd->timedout) { req->errors++; nbd_end_request(nbd, req); @@ -654,12 +660,13 @@ static int nbd_set_socket(struct nbd_device *
[PATCH 3/3]nbd: make nbd device wait for its users
When a timeout occurs or a recv fails, then instead of abruplty killing nbd block device wait for it's users to finish. This is more required when filesystem(s) like ext2 or ext3 don't expect their buffer heads to disappear while the filesystem is mounted. Each open of a nbd device is refcounted, while the userland program [nbd-client] doing the NBD_DO_IT ioctl would now wait for any other users of this device before invalidating the nbd device. A timedout or a disconnected device, if in use, can't be used until it has been resetted. The resetting happens when all tasks having this bdev open closes this bdev. Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 124 1 file changed, 96 insertions(+), 28 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 9223b09..0587bbd 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -70,10 +70,13 @@ struct nbd_device { #if IS_ENABLED(CONFIG_DEBUG_FS) struct dentry *dbg_dir; #endif + /* - *This is specifically for calling sock_shutdown, for now. - */ +*This is specifically for calling sock_shutdown, for now. +*/ struct work_struct ws_shutdown; + struct kref users; + struct completion user_completion; }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -104,6 +107,8 @@ static DEFINE_SPINLOCK(nbd_lock); * Shutdown function for nbd_dev work struct. */ static void nbd_ws_func_shutdown(struct work_struct *); +static void nbd_kref_release(struct kref *); +static int nbd_size_clear(struct nbd_device *, struct block_device *); static inline struct device *nbd_to_dev(struct nbd_device *nbd) { @@ -129,7 +134,7 @@ static const char *nbdcmd_to_ascii(int cmd) static int nbd_size_clear(struct nbd_device *nbd, struct block_device *bdev) { - bdev->bd_inode->i_size = 0; + i_size_write(bdev->bd_inode, 0); set_capacity(nbd->disk, 0); kobject_uevent(_to_dev(nbd)->kobj, KOBJ_CHANGE); @@ -141,7 +146,7 @@ static void nbd_size_update(struct nbd_device *nbd, struct block_device *bdev) if (!nbd_is_connected(nbd)) return; - bdev->bd_inode->i_size = nbd->bytesize; + i_size_write(bdev->bd_inode, nbd->bytesize); set_capacity(nbd->disk, nbd->bytesize >> 9); kobject_uevent(_to_dev(nbd)->kobj, KOBJ_CHANGE); } @@ -150,11 +155,9 @@ static int nbd_size_set(struct nbd_device *nbd, struct block_device *bdev, int blocksize, int nr_blocks) { int ret; - ret = set_blocksize(bdev, blocksize); if (ret) return ret; - nbd->blksize = blocksize; nbd->bytesize = (loff_t)blocksize * (loff_t)nr_blocks; @@ -202,14 +205,19 @@ static void nbd_xmit_timeout(unsigned long arg) { struct nbd_device *nbd = (struct nbd_device *)arg; + if (nbd->timedout) + return; + if (list_empty(>queue_head)) return; + nbd->timedout = true; - schedule_work(>ws_shutdown); + /* * Make sure sender thread sees nbd->timedout. */ smp_wmb(); + schedule_work(>ws_shutdown); wake_up(>waiting_wq); dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down connection\n"); } @@ -476,8 +484,6 @@ static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev) nbd_end_request(nbd, req); } - nbd_size_clear(nbd, bdev); - device_remove_file(disk_to_dev(nbd->disk), _attr_pid); nbd->task_recv = NULL; @@ -580,8 +586,8 @@ static int nbd_thread_send(void *data) while (!kthread_should_stop() || !list_empty(>waiting_queue)) { /* wait for something to do */ wait_event_interruptible(nbd->waiting_wq, - kthread_should_stop() || - !list_empty(>waiting_queue)); +kthread_should_stop() || +!list_empty(>waiting_queue)); /* extract request */ if (list_empty(>waiting_queue)) @@ -589,11 +595,11 @@ static int nbd_thread_send(void *data) spin_lock_irq(>queue_lock); req = list_entry(nbd->waiting_queue.next, struct request, - queuelist); +queuelist); list_del_init(>queuelist); spin_unlock_irq(>queue_lock); - nbd_handle_req(nbd, req); + /* handle request */ if (nbd->timedout) { req->errors++; nbd_end_request(nbd, req); @@ -654,12 +660,13 @@ static int nbd_set_socket(struct nbd_device *nbd, struct socket *so
[PATCH v3 1/3]nbd: fix might_sleep warning on socket shutdown
spinlocked ranges should be small and not contain calls into huge subfunctions. Fix my mistake and just get the pointer to the socket instead of doing everything with spinlock held. Reported-by: Mikulas Patocka <miku...@twibright.com> Signed-off-by: Markus Pargmann <m...@pengutronix.de> Changelog: Pranay Kr. Srivastava<pran...@gmail.com>: 1) Use spin_lock instead of irq version for sock_shutdown. 2) Use system work queue to actually trigger the shutdown of socket. This solves the issue when kernel_sendmsg is currently blocked while a timeout occurs. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 69 ++--- 1 file changed, 44 insertions(+), 25 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 56f7f5d..586d946 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -39,6 +39,7 @@ #include #include +#include struct nbd_device { u32 flags; @@ -69,6 +70,10 @@ struct nbd_device { #if IS_ENABLED(CONFIG_DEBUG_FS) struct dentry *dbg_dir; #endif + /* + *This is specifically for calling sock_shutdown, for now. + */ + struct work_struct ws_shutdown; }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -95,6 +100,11 @@ static int max_part; */ static DEFINE_SPINLOCK(nbd_lock); +/* + * Shutdown function for nbd_dev work struct. + */ +static void nbd_ws_func_shutdown(struct work_struct *); + static inline struct device *nbd_to_dev(struct nbd_device *nbd) { return disk_to_dev(nbd->disk); @@ -172,39 +182,35 @@ static void nbd_end_request(struct nbd_device *nbd, struct request *req) */ static void sock_shutdown(struct nbd_device *nbd) { - spin_lock_irq(>sock_lock); - - if (!nbd->sock) { - spin_unlock_irq(>sock_lock); - return; - } + struct socket *sock; - dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - sockfd_put(nbd->sock); + spin_lock(>sock_lock); + sock = nbd->sock; nbd->sock = NULL; - spin_unlock_irq(>sock_lock); + spin_unlock(>sock_lock); + + if (!sock) + return; del_timer(>timeout_timer); + dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); + kernel_sock_shutdown(sock, SHUT_RDWR); + sockfd_put(sock); } static void nbd_xmit_timeout(unsigned long arg) { struct nbd_device *nbd = (struct nbd_device *)arg; - unsigned long flags; if (list_empty(>queue_head)) return; - - spin_lock_irqsave(>sock_lock, flags); - nbd->timedout = true; - - if (nbd->sock) - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - - spin_unlock_irqrestore(>sock_lock, flags); - + schedule_work(>ws_shutdown); + /* +* Make sure sender thread sees nbd->timedout. +*/ + smp_wmb(); + wake_up(>waiting_wq); dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down connection\n"); } @@ -574,8 +580,8 @@ static int nbd_thread_send(void *data) while (!kthread_should_stop() || !list_empty(>waiting_queue)) { /* wait for something to do */ wait_event_interruptible(nbd->waiting_wq, -kthread_should_stop() || -!list_empty(>waiting_queue)); + kthread_should_stop() || + !list_empty(>waiting_queue)); /* extract request */ if (list_empty(>waiting_queue)) @@ -583,12 +589,16 @@ static int nbd_thread_send(void *data) spin_lock_irq(>queue_lock); req = list_entry(nbd->waiting_queue.next, struct request, -queuelist); + queuelist); list_del_init(>queuelist); spin_unlock_irq(>queue_lock); - /* handle request */ nbd_handle_req(nbd, req); + if (nbd->timedout) { + req->errors++; + nbd_end_request(nbd, req); + } else + nbd_handle_req(nbd, req); } nbd->task_send = NULL; @@ -668,6 +678,7 @@ static void nbd_reset(struct nbd_device *nbd) set_capacity(nbd->disk, 0); nbd->flags = 0; nbd->xmit_timeout = 0; + INIT_WORK(>ws_shutdown, nbd_ws_func_shutdown); queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue); del_timer_sync(>timeout_timer); } @@ -802,11 +813,11 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
[PATCH v3 1/3]nbd: fix might_sleep warning on socket shutdown
spinlocked ranges should be small and not contain calls into huge subfunctions. Fix my mistake and just get the pointer to the socket instead of doing everything with spinlock held. Reported-by: Mikulas Patocka Signed-off-by: Markus Pargmann Changelog: Pranay Kr. Srivastava: 1) Use spin_lock instead of irq version for sock_shutdown. 2) Use system work queue to actually trigger the shutdown of socket. This solves the issue when kernel_sendmsg is currently blocked while a timeout occurs. Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 69 ++--- 1 file changed, 44 insertions(+), 25 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 56f7f5d..586d946 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -39,6 +39,7 @@ #include #include +#include struct nbd_device { u32 flags; @@ -69,6 +70,10 @@ struct nbd_device { #if IS_ENABLED(CONFIG_DEBUG_FS) struct dentry *dbg_dir; #endif + /* + *This is specifically for calling sock_shutdown, for now. + */ + struct work_struct ws_shutdown; }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -95,6 +100,11 @@ static int max_part; */ static DEFINE_SPINLOCK(nbd_lock); +/* + * Shutdown function for nbd_dev work struct. + */ +static void nbd_ws_func_shutdown(struct work_struct *); + static inline struct device *nbd_to_dev(struct nbd_device *nbd) { return disk_to_dev(nbd->disk); @@ -172,39 +182,35 @@ static void nbd_end_request(struct nbd_device *nbd, struct request *req) */ static void sock_shutdown(struct nbd_device *nbd) { - spin_lock_irq(>sock_lock); - - if (!nbd->sock) { - spin_unlock_irq(>sock_lock); - return; - } + struct socket *sock; - dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - sockfd_put(nbd->sock); + spin_lock(>sock_lock); + sock = nbd->sock; nbd->sock = NULL; - spin_unlock_irq(>sock_lock); + spin_unlock(>sock_lock); + + if (!sock) + return; del_timer(>timeout_timer); + dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); + kernel_sock_shutdown(sock, SHUT_RDWR); + sockfd_put(sock); } static void nbd_xmit_timeout(unsigned long arg) { struct nbd_device *nbd = (struct nbd_device *)arg; - unsigned long flags; if (list_empty(>queue_head)) return; - - spin_lock_irqsave(>sock_lock, flags); - nbd->timedout = true; - - if (nbd->sock) - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - - spin_unlock_irqrestore(>sock_lock, flags); - + schedule_work(>ws_shutdown); + /* +* Make sure sender thread sees nbd->timedout. +*/ + smp_wmb(); + wake_up(>waiting_wq); dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down connection\n"); } @@ -574,8 +580,8 @@ static int nbd_thread_send(void *data) while (!kthread_should_stop() || !list_empty(>waiting_queue)) { /* wait for something to do */ wait_event_interruptible(nbd->waiting_wq, -kthread_should_stop() || -!list_empty(>waiting_queue)); + kthread_should_stop() || + !list_empty(>waiting_queue)); /* extract request */ if (list_empty(>waiting_queue)) @@ -583,12 +589,16 @@ static int nbd_thread_send(void *data) spin_lock_irq(>queue_lock); req = list_entry(nbd->waiting_queue.next, struct request, -queuelist); + queuelist); list_del_init(>queuelist); spin_unlock_irq(>queue_lock); - /* handle request */ nbd_handle_req(nbd, req); + if (nbd->timedout) { + req->errors++; + nbd_end_request(nbd, req); + } else + nbd_handle_req(nbd, req); } nbd->task_send = NULL; @@ -668,6 +678,7 @@ static void nbd_reset(struct nbd_device *nbd) set_capacity(nbd->disk, 0); nbd->flags = 0; nbd->xmit_timeout = 0; + INIT_WORK(>ws_shutdown, nbd_ws_func_shutdown); queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue); del_timer_sync(>timeout_timer); } @@ -802,11 +813,11 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, error = nbd_thread_recv(nbd, bdev); nbd_dev_dbg_close(nbd); kthread_stop
[PATCH v3 0/3] nbd: resolve bugs and limitations
This patch series fixes the following 1) fix might_sleep warning on socket shutdown: Fix sock_shutdown to avoid calling kernel_sock_shutdown while holding spin_lock. 2) cleanup nbd_set_socket Simple fixes to nbd_set_socket. 3) make nbd device wait for its users. When a timeout or error occurs then nbd driver simply kills the block device. Many filesystem(s) example ext2/ext3 don't expect their buffer heads to disappear like that. Fix this by making nbd device wait for its users. The same work function is used to trigger the kill_bdev as well do a sock_shutdown, depending on either a timeout/error occured or a disconnect was issued. Also avoid scheduling the work_fn in case a timeout for a request has already occured. Pranay Kr. Srivastava (3): fix might_sleep warning on socket shutdown cleanup nbd_set_socket make nbd device wait for its users drivers/block/nbd.c | 169 +++- 1 file changed, 127 insertions(+), 42 deletions(-) Changelog: Rebased all patches above on git://git.pengutronix.de/git/mpa/linux-nbd.git, commit:7ed71a8704eda7b75fbd0ed73fd0a5b6e469d250 3) make nbd device wait for its users. Instead of using kref_sub just use kref_init and under the bd_mutex for serializing on open/close. -- 1.9.1
[PATCH v3 0/3] nbd: resolve bugs and limitations
This patch series fixes the following 1) fix might_sleep warning on socket shutdown: Fix sock_shutdown to avoid calling kernel_sock_shutdown while holding spin_lock. 2) cleanup nbd_set_socket Simple fixes to nbd_set_socket. 3) make nbd device wait for its users. When a timeout or error occurs then nbd driver simply kills the block device. Many filesystem(s) example ext2/ext3 don't expect their buffer heads to disappear like that. Fix this by making nbd device wait for its users. The same work function is used to trigger the kill_bdev as well do a sock_shutdown, depending on either a timeout/error occured or a disconnect was issued. Also avoid scheduling the work_fn in case a timeout for a request has already occured. Pranay Kr. Srivastava (3): fix might_sleep warning on socket shutdown cleanup nbd_set_socket make nbd device wait for its users drivers/block/nbd.c | 169 +++- 1 file changed, 127 insertions(+), 42 deletions(-) Changelog: Rebased all patches above on git://git.pengutronix.de/git/mpa/linux-nbd.git, commit:7ed71a8704eda7b75fbd0ed73fd0a5b6e469d250 3) make nbd device wait for its users. Instead of using kref_sub just use kref_init and under the bd_mutex for serializing on open/close. -- 1.9.1
[PATCH v2 0/5] nbd: fixes for nbd
This patch series fixes the following 1) fix might_sleep warning on socket shutdown: Fix sock_shutdown to avoid calling kernel_sock_shutdown while holding spin_lock. 2) cleanup nbd_set_socket Cleanup nbd_set_socket to use spin_lock instead of irq version and remove the goto statement in favour of a simple if-else statement. 3) fix various coding standard warnings Make shutdown get called in a process context instead, using system_wq. 4) make nbd device wait for its users. When a timeout or error occurs then nbd driver simply kills the block device. Many filesystem(s) example ext2/ext3 don't expect their buffer heads to disappear like that. Fix this by making nbd device wait for its users. Introduced a new field to check if the device is currently in use or not. This helps to check if the kref_put should be done on device release or not. This field needs to be atomic as the release function may be called from NBD_DO_IT as well as from device's release function. 5) use device_attr macros for sysfs attribute use DEVICE_ATTR_RO for sysfs pid attribute. Changelog for v2: 1) fix might_sleep warning on socket shutdown use bool timedout instead of atomic 2) cleanup nbd_set_socket Added this new patch to this series. 3) fix various coding standard warnings No Change. 4) make nbd device wait for its users Earlier version used to do a final kref put when the kref->counter == 2. This required a check of the internal atomic counter of kref which was ugly. v2 of this patch make this more readable and doesn't do manual check of the internal counter used by kref. 5) use device_attr macros for sysfs attribute No Change. Pranay Kr. Srivastava (5): fix might_sleep warning on socket shutdown. cleanup nbd_set_socket fix various coding standard warnings make nbd device wait for its users. use device_attr macros for sysfs attribute drivers/block/nbd.c | 173 +--- 1 file changed, 124 insertions(+), 49 deletions(-) -- 2.6.2
[PATCH v2 0/5] nbd: fixes for nbd
This patch series fixes the following 1) fix might_sleep warning on socket shutdown: Fix sock_shutdown to avoid calling kernel_sock_shutdown while holding spin_lock. 2) cleanup nbd_set_socket Cleanup nbd_set_socket to use spin_lock instead of irq version and remove the goto statement in favour of a simple if-else statement. 3) fix various coding standard warnings Make shutdown get called in a process context instead, using system_wq. 4) make nbd device wait for its users. When a timeout or error occurs then nbd driver simply kills the block device. Many filesystem(s) example ext2/ext3 don't expect their buffer heads to disappear like that. Fix this by making nbd device wait for its users. Introduced a new field to check if the device is currently in use or not. This helps to check if the kref_put should be done on device release or not. This field needs to be atomic as the release function may be called from NBD_DO_IT as well as from device's release function. 5) use device_attr macros for sysfs attribute use DEVICE_ATTR_RO for sysfs pid attribute. Changelog for v2: 1) fix might_sleep warning on socket shutdown use bool timedout instead of atomic 2) cleanup nbd_set_socket Added this new patch to this series. 3) fix various coding standard warnings No Change. 4) make nbd device wait for its users Earlier version used to do a final kref put when the kref->counter == 2. This required a check of the internal atomic counter of kref which was ugly. v2 of this patch make this more readable and doesn't do manual check of the internal counter used by kref. 5) use device_attr macros for sysfs attribute No Change. Pranay Kr. Srivastava (5): fix might_sleep warning on socket shutdown. cleanup nbd_set_socket fix various coding standard warnings make nbd device wait for its users. use device_attr macros for sysfs attribute drivers/block/nbd.c | 173 +--- 1 file changed, 124 insertions(+), 49 deletions(-) -- 2.6.2
[PATCH v2 1/5] nbd: fix might_sleep warning on socket shutdown.
spinlocked ranges should be small and not contain calls into huge subfunctions. Fix my mistake and just get the pointer to the socket instead of doing everything with spinlock held. Reported-by: Mikulas Patocka <miku...@twibright.com> Signed-off-by: Markus Pargmann <m...@pengutronix.de> Changelog: Pranay Kr. Srivastava<pran...@gmail.com>: 1) Use spin_lock instead of irq version for sock_shutdown. 2) Use system work queue to actually trigger the shutdown of socket. This solves the issue when kernel_sendmsg is currently blocked while a timeout occurs. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 65 ++--- 1 file changed, 42 insertions(+), 23 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 31e73a7..0339d40 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -39,6 +39,7 @@ #include #include +#include struct nbd_device { u32 flags; @@ -69,6 +70,10 @@ struct nbd_device { #if IS_ENABLED(CONFIG_DEBUG_FS) struct dentry *dbg_dir; #endif + /* +*This is specifically for calling sock_shutdown, for now. +*/ + struct work_struct ws_shutdown; }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -95,6 +100,11 @@ static int max_part; */ static DEFINE_SPINLOCK(nbd_lock); +/* + * Shutdown function for nbd_dev work struct. + */ +static void nbd_ws_func_shutdown(struct work_struct *); + static inline struct device *nbd_to_dev(struct nbd_device *nbd) { return disk_to_dev(nbd->disk); @@ -172,39 +182,35 @@ static void nbd_end_request(struct nbd_device *nbd, struct request *req) */ static void sock_shutdown(struct nbd_device *nbd) { - spin_lock_irq(>sock_lock); - - if (!nbd->sock) { - spin_unlock_irq(>sock_lock); - return; - } + struct socket *sock; - dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - sockfd_put(nbd->sock); + spin_lock(>sock_lock); + sock = nbd->sock; nbd->sock = NULL; - spin_unlock_irq(>sock_lock); + spin_unlock(>sock_lock); + + if (!sock) + return; del_timer(>timeout_timer); + dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); + kernel_sock_shutdown(sock, SHUT_RDWR); + sockfd_put(sock); } static void nbd_xmit_timeout(unsigned long arg) { struct nbd_device *nbd = (struct nbd_device *)arg; - unsigned long flags; if (list_empty(>queue_head)) return; - - spin_lock_irqsave(>sock_lock, flags); - nbd->timedout = true; - - if (nbd->sock) - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - - spin_unlock_irqrestore(>sock_lock, flags); - + schedule_work(>ws_shutdown); + /* +* Make sure sender thread sees nbd->timedout. +*/ + smp_wmb(); + wake_up(>waiting_wq); dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down connection\n"); } @@ -592,7 +598,11 @@ static int nbd_thread_send(void *data) spin_unlock_irq(>queue_lock); /* handle request */ - nbd_handle_req(nbd, req); + if (nbd->timedout) { + req->errors++; + nbd_end_request(nbd, req); + } else + nbd_handle_req(nbd, req); } nbd->task_send = NULL; @@ -672,6 +682,7 @@ static void nbd_reset(struct nbd_device *nbd) set_capacity(nbd->disk, 0); nbd->flags = 0; nbd->xmit_timeout = 0; + INIT_WORK(>ws_shutdown, nbd_ws_func_shutdown); queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue); del_timer_sync(>timeout_timer); } @@ -804,15 +815,15 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, nbd_dev_dbg_close(nbd); kthread_stop(thread); - mutex_lock(>tx_lock); - sock_shutdown(nbd); + mutex_lock(>tx_lock); nbd_clear_que(nbd); kill_bdev(bdev); nbd_bdev_reset(bdev); if (nbd->disconnect) /* user requested, ignore socket errors */ error = 0; + if (nbd->timedout) error = -ETIMEDOUT; @@ -863,6 +874,14 @@ static const struct block_device_operations nbd_fops = .compat_ioctl = nbd_ioctl, }; +static void nbd_ws_func_shutdown(struct work_struct *ws_nbd) +{ + struct nbd_device *nbd_dev = container_of(ws_nbd, struct nbd_device, + ws_shutdown); + + sock_shutdown(nbd_dev); +} + #if IS_ENABLED(CONFIG_DEBUG_FS) static int nbd_dbg_tasks_show(struct seq_file *s, void *unused) -- 2.6.2
[PATCH v2 1/5] nbd: fix might_sleep warning on socket shutdown.
spinlocked ranges should be small and not contain calls into huge subfunctions. Fix my mistake and just get the pointer to the socket instead of doing everything with spinlock held. Reported-by: Mikulas Patocka Signed-off-by: Markus Pargmann Changelog: Pranay Kr. Srivastava: 1) Use spin_lock instead of irq version for sock_shutdown. 2) Use system work queue to actually trigger the shutdown of socket. This solves the issue when kernel_sendmsg is currently blocked while a timeout occurs. Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 65 ++--- 1 file changed, 42 insertions(+), 23 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 31e73a7..0339d40 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -39,6 +39,7 @@ #include #include +#include struct nbd_device { u32 flags; @@ -69,6 +70,10 @@ struct nbd_device { #if IS_ENABLED(CONFIG_DEBUG_FS) struct dentry *dbg_dir; #endif + /* +*This is specifically for calling sock_shutdown, for now. +*/ + struct work_struct ws_shutdown; }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -95,6 +100,11 @@ static int max_part; */ static DEFINE_SPINLOCK(nbd_lock); +/* + * Shutdown function for nbd_dev work struct. + */ +static void nbd_ws_func_shutdown(struct work_struct *); + static inline struct device *nbd_to_dev(struct nbd_device *nbd) { return disk_to_dev(nbd->disk); @@ -172,39 +182,35 @@ static void nbd_end_request(struct nbd_device *nbd, struct request *req) */ static void sock_shutdown(struct nbd_device *nbd) { - spin_lock_irq(>sock_lock); - - if (!nbd->sock) { - spin_unlock_irq(>sock_lock); - return; - } + struct socket *sock; - dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - sockfd_put(nbd->sock); + spin_lock(>sock_lock); + sock = nbd->sock; nbd->sock = NULL; - spin_unlock_irq(>sock_lock); + spin_unlock(>sock_lock); + + if (!sock) + return; del_timer(>timeout_timer); + dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); + kernel_sock_shutdown(sock, SHUT_RDWR); + sockfd_put(sock); } static void nbd_xmit_timeout(unsigned long arg) { struct nbd_device *nbd = (struct nbd_device *)arg; - unsigned long flags; if (list_empty(>queue_head)) return; - - spin_lock_irqsave(>sock_lock, flags); - nbd->timedout = true; - - if (nbd->sock) - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - - spin_unlock_irqrestore(>sock_lock, flags); - + schedule_work(>ws_shutdown); + /* +* Make sure sender thread sees nbd->timedout. +*/ + smp_wmb(); + wake_up(>waiting_wq); dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down connection\n"); } @@ -592,7 +598,11 @@ static int nbd_thread_send(void *data) spin_unlock_irq(>queue_lock); /* handle request */ - nbd_handle_req(nbd, req); + if (nbd->timedout) { + req->errors++; + nbd_end_request(nbd, req); + } else + nbd_handle_req(nbd, req); } nbd->task_send = NULL; @@ -672,6 +682,7 @@ static void nbd_reset(struct nbd_device *nbd) set_capacity(nbd->disk, 0); nbd->flags = 0; nbd->xmit_timeout = 0; + INIT_WORK(>ws_shutdown, nbd_ws_func_shutdown); queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue); del_timer_sync(>timeout_timer); } @@ -804,15 +815,15 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, nbd_dev_dbg_close(nbd); kthread_stop(thread); - mutex_lock(>tx_lock); - sock_shutdown(nbd); + mutex_lock(>tx_lock); nbd_clear_que(nbd); kill_bdev(bdev); nbd_bdev_reset(bdev); if (nbd->disconnect) /* user requested, ignore socket errors */ error = 0; + if (nbd->timedout) error = -ETIMEDOUT; @@ -863,6 +874,14 @@ static const struct block_device_operations nbd_fops = .compat_ioctl = nbd_ioctl, }; +static void nbd_ws_func_shutdown(struct work_struct *ws_nbd) +{ + struct nbd_device *nbd_dev = container_of(ws_nbd, struct nbd_device, + ws_shutdown); + + sock_shutdown(nbd_dev); +} + #if IS_ENABLED(CONFIG_DEBUG_FS) static int nbd_dbg_tasks_show(struct seq_file *s, void *unused) -- 2.6.2
[PATCH v2 4/5]nbd: make nbd device wait for its users.
When a timeout occurs or a recv fails, then instead of abruplty killing nbd block device wait for it's users to finish. This is more required when filesystem(s) like ext2 or ext3 don't expect their buffer heads to disappear while the filesystem is mounted. Each open of a nbd device is refcounted, while the userland program [nbd-client] doing the NBD_DO_IT ioctl would now wait for any other users of this device before invalidating the nbd device. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 58 + 1 file changed, 58 insertions(+) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index d1d898d..4da40dc 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -70,10 +70,13 @@ struct nbd_device { #if IS_ENABLED(CONFIG_DEBUG_FS) struct dentry *dbg_dir; #endif + atomic_t inuse; /* *This is specifically for calling sock_shutdown, for now. */ struct work_struct ws_shutdown; + struct kref users; + struct completion user_completion; }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -104,6 +107,7 @@ static DEFINE_SPINLOCK(nbd_lock); * Shutdown function for nbd_dev work struct. */ static void nbd_ws_func_shutdown(struct work_struct *); +static void nbd_kref_release(struct kref *); static inline struct device *nbd_to_dev(struct nbd_device *nbd) { @@ -682,6 +686,8 @@ static void nbd_reset(struct nbd_device *nbd) nbd->flags = 0; nbd->xmit_timeout = 0; INIT_WORK(>ws_shutdown, nbd_ws_func_shutdown); + init_completion(>user_completion); + kref_init(>users); queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue); del_timer_sync(>timeout_timer); } @@ -815,6 +821,14 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, kthread_stop(thread); sock_shutdown(nbd); + /* +* kref_init initializes with ref count as 1, +* nbd_client, or the user-land program executing +* this ioctl will make the refcount to 2[at least] +* so subtracting 2 from refcount. +*/ + kref_sub(>users, 2, nbd_kref_release); + wait_for_completion(>user_completion); mutex_lock(>tx_lock); nbd_clear_que(nbd); kill_bdev(bdev); @@ -865,13 +879,56 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t mode, return error; } +static void nbd_kref_release(struct kref *kref_users) +{ + struct nbd_device *nbd = container_of(kref_users, struct nbd_device, + users); + pr_debug("Releasing kref [%s]\n", __func__); + atomic_set(>inuse, 0); + complete(>user_completion); + +} + +static int nbd_open(struct block_device *bdev, fmode_t mode) +{ + struct nbd_device *nbd_dev = bdev->bd_disk->private_data; + + if (kref_get_unless_zero(_dev->users)) + atomic_set(_dev->inuse, 1); + + pr_debug("Opening nbd_dev %s. Active users = %u\n", + bdev->bd_disk->disk_name, + atomic_read(_dev->users.refcount) - 1); + return 0; +} + +static void nbd_release(struct gendisk *disk, fmode_t mode) +{ + struct nbd_device *nbd_dev = disk->private_data; + /* + *kref_init initializes ref count to 1, so we + *we check for refcount to be 2 for a final put. + * + *kref needs to be re-initialized just here as the + *other process holding it must see the ref count as 2. + */ + if (atomic_read(_dev->inuse)) + kref_put(_dev->users, nbd_kref_release); + + pr_debug("Closing nbd_dev %s. Active users = %u\n", + disk->disk_name, + atomic_read(_dev->users.refcount) - 1); +} static const struct block_device_operations nbd_fops = { .owner =THIS_MODULE, .ioctl =nbd_ioctl, .compat_ioctl = nbd_ioctl, + .open = nbd_open, + .release = nbd_release }; + static void nbd_ws_func_shutdown(struct work_struct *ws_nbd) { struct nbd_device *nbd_dev = container_of(ws_nbd, struct nbd_device, @@ -1107,6 +1164,7 @@ static int __init nbd_init(void) disk->fops = _fops; disk->private_data = _dev[i]; sprintf(disk->disk_name, "nbd%d", i); + atomic_set(_dev[i].inuse, 0); nbd_reset(_dev[i]); add_disk(disk); } -- 2.6.2
[PATCH v2 4/5]nbd: make nbd device wait for its users.
When a timeout occurs or a recv fails, then instead of abruplty killing nbd block device wait for it's users to finish. This is more required when filesystem(s) like ext2 or ext3 don't expect their buffer heads to disappear while the filesystem is mounted. Each open of a nbd device is refcounted, while the userland program [nbd-client] doing the NBD_DO_IT ioctl would now wait for any other users of this device before invalidating the nbd device. Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 58 + 1 file changed, 58 insertions(+) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index d1d898d..4da40dc 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -70,10 +70,13 @@ struct nbd_device { #if IS_ENABLED(CONFIG_DEBUG_FS) struct dentry *dbg_dir; #endif + atomic_t inuse; /* *This is specifically for calling sock_shutdown, for now. */ struct work_struct ws_shutdown; + struct kref users; + struct completion user_completion; }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -104,6 +107,7 @@ static DEFINE_SPINLOCK(nbd_lock); * Shutdown function for nbd_dev work struct. */ static void nbd_ws_func_shutdown(struct work_struct *); +static void nbd_kref_release(struct kref *); static inline struct device *nbd_to_dev(struct nbd_device *nbd) { @@ -682,6 +686,8 @@ static void nbd_reset(struct nbd_device *nbd) nbd->flags = 0; nbd->xmit_timeout = 0; INIT_WORK(>ws_shutdown, nbd_ws_func_shutdown); + init_completion(>user_completion); + kref_init(>users); queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue); del_timer_sync(>timeout_timer); } @@ -815,6 +821,14 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, kthread_stop(thread); sock_shutdown(nbd); + /* +* kref_init initializes with ref count as 1, +* nbd_client, or the user-land program executing +* this ioctl will make the refcount to 2[at least] +* so subtracting 2 from refcount. +*/ + kref_sub(>users, 2, nbd_kref_release); + wait_for_completion(>user_completion); mutex_lock(>tx_lock); nbd_clear_que(nbd); kill_bdev(bdev); @@ -865,13 +879,56 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t mode, return error; } +static void nbd_kref_release(struct kref *kref_users) +{ + struct nbd_device *nbd = container_of(kref_users, struct nbd_device, + users); + pr_debug("Releasing kref [%s]\n", __func__); + atomic_set(>inuse, 0); + complete(>user_completion); + +} + +static int nbd_open(struct block_device *bdev, fmode_t mode) +{ + struct nbd_device *nbd_dev = bdev->bd_disk->private_data; + + if (kref_get_unless_zero(_dev->users)) + atomic_set(_dev->inuse, 1); + + pr_debug("Opening nbd_dev %s. Active users = %u\n", + bdev->bd_disk->disk_name, + atomic_read(_dev->users.refcount) - 1); + return 0; +} + +static void nbd_release(struct gendisk *disk, fmode_t mode) +{ + struct nbd_device *nbd_dev = disk->private_data; + /* + *kref_init initializes ref count to 1, so we + *we check for refcount to be 2 for a final put. + * + *kref needs to be re-initialized just here as the + *other process holding it must see the ref count as 2. + */ + if (atomic_read(_dev->inuse)) + kref_put(_dev->users, nbd_kref_release); + + pr_debug("Closing nbd_dev %s. Active users = %u\n", + disk->disk_name, + atomic_read(_dev->users.refcount) - 1); +} static const struct block_device_operations nbd_fops = { .owner =THIS_MODULE, .ioctl =nbd_ioctl, .compat_ioctl = nbd_ioctl, + .open = nbd_open, + .release = nbd_release }; + static void nbd_ws_func_shutdown(struct work_struct *ws_nbd) { struct nbd_device *nbd_dev = container_of(ws_nbd, struct nbd_device, @@ -1107,6 +1164,7 @@ static int __init nbd_init(void) disk->fops = _fops; disk->private_data = _dev[i]; sprintf(disk->disk_name, "nbd%d", i); + atomic_set(_dev[i].inuse, 0); nbd_reset(_dev[i]); add_disk(disk); } -- 2.6.2
[PATCH v2 5/5]nbd: use device_attr macros for sysfs attribute
This patch changes the pid sysfs device attribute to use DEVICE_ATTR_* macro. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 4da40dc..323ab26 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -449,10 +449,8 @@ static ssize_t pid_show(struct device *dev, return sprintf(buf, "%d\n", task_pid_nr(nbd->task_recv)); } -static struct device_attribute pid_attr = { - .attr = { .name = "pid", .mode = S_IRUGO}, - .show = pid_show, -}; + +static DEVICE_ATTR_RO(pid); static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev) { @@ -465,7 +463,7 @@ static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev) nbd->task_recv = current; - ret = device_create_file(disk_to_dev(nbd->disk), _attr); + ret = device_create_file(disk_to_dev(nbd->disk), _attr_pid); if (ret) { dev_err(disk_to_dev(nbd->disk), "device_create_file failed!\n"); @@ -488,7 +486,7 @@ static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev) nbd_size_clear(nbd, bdev); - device_remove_file(disk_to_dev(nbd->disk), _attr); + device_remove_file(disk_to_dev(nbd->disk), _attr_pid); nbd->task_recv = NULL; -- 2.6.2
[PATCH v2 2/5]nbd: cleanup nbd_set_socket
This patch 1) uses spin_lock instead of irq version. 2) removes the goto statement in case a socket is already assigned with simple if-else statement. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 0339d40..da2b0a4 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -657,17 +657,14 @@ static int nbd_set_socket(struct nbd_device *nbd, struct socket *sock) { int ret = 0; - spin_lock_irq(>sock_lock); + spin_lock(>sock_lock); - if (nbd->sock) { + if (nbd->sock) ret = -EBUSY; - goto out; - } - - nbd->sock = sock; + else + nbd->sock = sock; -out: - spin_unlock_irq(>sock_lock); + spin_unlock(>sock_lock); return ret; } -- 2.6.2
[PATCH v2 5/5]nbd: use device_attr macros for sysfs attribute
This patch changes the pid sysfs device attribute to use DEVICE_ATTR_* macro. Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 4da40dc..323ab26 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -449,10 +449,8 @@ static ssize_t pid_show(struct device *dev, return sprintf(buf, "%d\n", task_pid_nr(nbd->task_recv)); } -static struct device_attribute pid_attr = { - .attr = { .name = "pid", .mode = S_IRUGO}, - .show = pid_show, -}; + +static DEVICE_ATTR_RO(pid); static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev) { @@ -465,7 +463,7 @@ static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev) nbd->task_recv = current; - ret = device_create_file(disk_to_dev(nbd->disk), _attr); + ret = device_create_file(disk_to_dev(nbd->disk), _attr_pid); if (ret) { dev_err(disk_to_dev(nbd->disk), "device_create_file failed!\n"); @@ -488,7 +486,7 @@ static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev) nbd_size_clear(nbd, bdev); - device_remove_file(disk_to_dev(nbd->disk), _attr); + device_remove_file(disk_to_dev(nbd->disk), _attr_pid); nbd->task_recv = NULL; -- 2.6.2
[PATCH v2 2/5]nbd: cleanup nbd_set_socket
This patch 1) uses spin_lock instead of irq version. 2) removes the goto statement in case a socket is already assigned with simple if-else statement. Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 0339d40..da2b0a4 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -657,17 +657,14 @@ static int nbd_set_socket(struct nbd_device *nbd, struct socket *sock) { int ret = 0; - spin_lock_irq(>sock_lock); + spin_lock(>sock_lock); - if (nbd->sock) { + if (nbd->sock) ret = -EBUSY; - goto out; - } - - nbd->sock = sock; + else + nbd->sock = sock; -out: - spin_unlock_irq(>sock_lock); + spin_unlock(>sock_lock); return ret; } -- 2.6.2
[PATCH v2 3/5]nbd: fix various coding standard warnings
1 )nbd: fix checkpatch trailing space warning. 2) nbd: fix checkpatch warning use linux/uaccess.h 3) nbd : fix checkpatch pointer declaration warning 4) nbd: fix checkpatch warning no newline after decleration. 5) nbd: fix checkpatch warning no newline after decleration. 6) nbd : fix checkpatch line over 80 char warning 7) nbd: fix checkpatch trailing whitespace warning. 8) nbd: fix checkpatch trailing whitespace warning. 9) nbd : fix checkpatch structure declaration braces on next line warning. 10) nbd : fix checkpatch trailing whitespace warning 11) nbd : fix checkpatch printk warning 12) nbd: fix checkpatch no extra line after decleration warning 13) nbd: fix checkpatch printk warning to pr_info 14) nbd: fix checkpatch no new line after decleration warning 15) nbd: fix checkpatch printk warning to pr_info Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 29 - 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index da2b0a4..d1d898d 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -3,7 +3,7 @@ * * Note that you can not swap over this thing, yet. Seems to work but * deadlocks sometimes - you can not swap over TCP in general. - * + * * Copyright 1997-2000, 2008 Pavel Machek <pa...@ucw.cz> * Parts copyright 2001 Steven Whitehouse <st...@chygwyn.com> * @@ -35,7 +35,7 @@ #include #include -#include +#include #include #include @@ -43,7 +43,7 @@ struct nbd_device { u32 flags; - struct socket * sock; /* If == NULL, device is not ready, yet */ + struct socket *sock;/* If == NULL, device is not ready, yet */ int magic; spinlock_t queue_lock; @@ -272,6 +272,7 @@ static inline int sock_send_bvec(struct nbd_device *nbd, struct bio_vec *bvec, { int result; void *kaddr = kmap(bvec->bv_page); + result = sock_xmit(nbd, 1, kaddr + bvec->bv_offset, bvec->bv_len, flags); kunmap(bvec->bv_page); @@ -369,6 +370,7 @@ static inline int sock_recv_bvec(struct nbd_device *nbd, struct bio_vec *bvec) { int result; void *kaddr = kmap(bvec->bv_page); + result = sock_xmit(nbd, 0, kaddr + bvec->bv_offset, bvec->bv_len, MSG_WAITALL); kunmap(bvec->bv_page); @@ -611,8 +613,8 @@ static int nbd_thread_send(void *data) } /* - * We always wait for result of write, for now. It would be nice to make it optional - * in future + * We always wait for result of write, for now. It would be nice to make it + * optional in future * if ((rq_data_dir(req) == WRITE) && (nbd->flags & NBD_WRITE_NOCHK)) * { printk( "Warning: Ignoring result!\n"); nbd_end_request( req ); } */ @@ -621,7 +623,7 @@ static void nbd_request_handler(struct request_queue *q) __releases(q->queue_lock) __acquires(q->queue_lock) { struct request *req; - + while ((req = blk_fetch_request(q)) != NULL) { struct nbd_device *nbd; @@ -737,7 +739,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, nbd_send_req(nbd, ); return 0; } - + case NBD_CLEAR_SOCK: sock_shutdown(nbd); nbd_clear_que(nbd); @@ -864,8 +866,7 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t mode, return error; } -static const struct block_device_operations nbd_fops = -{ +static const struct block_device_operations nbd_fops = { .owner =THIS_MODULE, .ioctl =nbd_ioctl, .compat_ioctl = nbd_ioctl, @@ -1008,7 +1009,7 @@ static void nbd_dbg_close(void) #endif /* - * And here should be modules and kernel interface + * And here should be modules and kernel interface * (Just smiley confuses emacs :-) */ @@ -1021,7 +1022,7 @@ static int __init nbd_init(void) BUILD_BUG_ON(sizeof(struct nbd_request) != 28); if (max_part < 0) { - printk(KERN_ERR "nbd: max_part must be >= 0\n"); + pr_err("nbd: max_part must be >= 0\n"); return -EINVAL; } @@ -1052,6 +1053,7 @@ static int __init nbd_init(void) for (i = 0; i < nbds_max; i++) { struct gendisk *disk = alloc_disk(1 << part_shift); + if (!disk) goto out; nbd_dev[i].disk = disk; @@ -1082,12 +1084,13 @@ static int __init nbd_init(void) goto out; } - printk(KERN_INFO "nbd: registered device at major %d\n", NBD_MAJOR); + pr_info("nbd: registered device at major %d\n", NBD_MAJOR); nbd_dbg_init(); for (i = 0; i < nbds_max; i++) { struct
[PATCH v2 3/5]nbd: fix various coding standard warnings
1 )nbd: fix checkpatch trailing space warning. 2) nbd: fix checkpatch warning use linux/uaccess.h 3) nbd : fix checkpatch pointer declaration warning 4) nbd: fix checkpatch warning no newline after decleration. 5) nbd: fix checkpatch warning no newline after decleration. 6) nbd : fix checkpatch line over 80 char warning 7) nbd: fix checkpatch trailing whitespace warning. 8) nbd: fix checkpatch trailing whitespace warning. 9) nbd : fix checkpatch structure declaration braces on next line warning. 10) nbd : fix checkpatch trailing whitespace warning 11) nbd : fix checkpatch printk warning 12) nbd: fix checkpatch no extra line after decleration warning 13) nbd: fix checkpatch printk warning to pr_info 14) nbd: fix checkpatch no new line after decleration warning 15) nbd: fix checkpatch printk warning to pr_info Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 29 - 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index da2b0a4..d1d898d 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -3,7 +3,7 @@ * * Note that you can not swap over this thing, yet. Seems to work but * deadlocks sometimes - you can not swap over TCP in general. - * + * * Copyright 1997-2000, 2008 Pavel Machek * Parts copyright 2001 Steven Whitehouse * @@ -35,7 +35,7 @@ #include #include -#include +#include #include #include @@ -43,7 +43,7 @@ struct nbd_device { u32 flags; - struct socket * sock; /* If == NULL, device is not ready, yet */ + struct socket *sock;/* If == NULL, device is not ready, yet */ int magic; spinlock_t queue_lock; @@ -272,6 +272,7 @@ static inline int sock_send_bvec(struct nbd_device *nbd, struct bio_vec *bvec, { int result; void *kaddr = kmap(bvec->bv_page); + result = sock_xmit(nbd, 1, kaddr + bvec->bv_offset, bvec->bv_len, flags); kunmap(bvec->bv_page); @@ -369,6 +370,7 @@ static inline int sock_recv_bvec(struct nbd_device *nbd, struct bio_vec *bvec) { int result; void *kaddr = kmap(bvec->bv_page); + result = sock_xmit(nbd, 0, kaddr + bvec->bv_offset, bvec->bv_len, MSG_WAITALL); kunmap(bvec->bv_page); @@ -611,8 +613,8 @@ static int nbd_thread_send(void *data) } /* - * We always wait for result of write, for now. It would be nice to make it optional - * in future + * We always wait for result of write, for now. It would be nice to make it + * optional in future * if ((rq_data_dir(req) == WRITE) && (nbd->flags & NBD_WRITE_NOCHK)) * { printk( "Warning: Ignoring result!\n"); nbd_end_request( req ); } */ @@ -621,7 +623,7 @@ static void nbd_request_handler(struct request_queue *q) __releases(q->queue_lock) __acquires(q->queue_lock) { struct request *req; - + while ((req = blk_fetch_request(q)) != NULL) { struct nbd_device *nbd; @@ -737,7 +739,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, nbd_send_req(nbd, ); return 0; } - + case NBD_CLEAR_SOCK: sock_shutdown(nbd); nbd_clear_que(nbd); @@ -864,8 +866,7 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t mode, return error; } -static const struct block_device_operations nbd_fops = -{ +static const struct block_device_operations nbd_fops = { .owner =THIS_MODULE, .ioctl =nbd_ioctl, .compat_ioctl = nbd_ioctl, @@ -1008,7 +1009,7 @@ static void nbd_dbg_close(void) #endif /* - * And here should be modules and kernel interface + * And here should be modules and kernel interface * (Just smiley confuses emacs :-) */ @@ -1021,7 +1022,7 @@ static int __init nbd_init(void) BUILD_BUG_ON(sizeof(struct nbd_request) != 28); if (max_part < 0) { - printk(KERN_ERR "nbd: max_part must be >= 0\n"); + pr_err("nbd: max_part must be >= 0\n"); return -EINVAL; } @@ -1052,6 +1053,7 @@ static int __init nbd_init(void) for (i = 0; i < nbds_max; i++) { struct gendisk *disk = alloc_disk(1 << part_shift); + if (!disk) goto out; nbd_dev[i].disk = disk; @@ -1082,12 +1084,13 @@ static int __init nbd_init(void) goto out; } - printk(KERN_INFO "nbd: registered device at major %d\n", NBD_MAJOR); + pr_info("nbd: registered device at major %d\n", NBD_MAJOR); nbd_dbg_init(); for (i = 0; i < nbds_max; i++) { struct gendisk *disk = nbd_dev[i].disk; + nbd_dev[i].magic = NBD_MAGIC;
[PATCH 3/4] make nbd device wait for its users.
When a timeout occurs or a recv fails, then instead of abruplty killing nbd block device wait for it's users to finish. This is more required when filesystem(s) like ext2 or ext3 don't expect their buffer heads to disappear while the filesystem is mounted. Use a kref for users using this. The device will be released for kref count of 2, not less or more. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 51 +++ 1 file changed, 51 insertions(+) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index af86c9b..59db890 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -71,6 +71,8 @@ struct nbd_device { struct dentry *dbg_dir; #endif struct work_struct ws_nbd; + struct kref users; + struct completion user_completion; }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -674,6 +676,7 @@ static void nbd_reset(struct nbd_device *nbd) nbd->flags = 0; nbd->xmit_timeout = 0; INIT_WORK(>ws_nbd, nbd_work_func); + init_completion(>user_completion); queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue); del_timer_sync(>timeout_timer); } @@ -807,6 +810,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, kthread_stop(thread); sock_shutdown(nbd); + wait_for_completion(>user_completion); mutex_lock(>tx_lock); nbd_clear_que(nbd); kill_bdev(bdev); @@ -858,12 +862,58 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t mode, return error; } +static void nbd_kref_release(struct kref *kref_users) +{ + struct nbd_device *nbd = container_of(kref_users, struct nbd_device, + users); + pr_debug("Releasing kref [%s]\n", __FUNCTION__); + complete(>user_completion); + +} + +static int nbd_open(struct block_device *bdev, fmode_t mode) +{ + struct nbd_device *nbd_dev = bdev->bd_disk->private_data; + + kref_get(_dev->users); + pr_debug("Opening nbd_dev %s. Active users = %u\n", + bdev->bd_disk->disk_name, + atomic_read(_dev->users.refcount) - 1); + return 0; +} + +static void nbd_release(struct gendisk *disk, fmode_t mode) +{ + struct nbd_device *nbd_dev = disk->private_data; + /* + *kref_init initializes ref count to 1, so we + *we check for refcount to be 2 for a final put. + * + *kref needs to be re-initialized just here as the + *other process holding it must see the ref count as 2. + */ + kref_put(_dev->users, nbd_kref_release); + + if (atomic_read(_dev->users.refcount) == 2) { + kref_sub(_dev->users, 2, nbd_kref_release); + kref_init(_dev->users); + kref_get(_dev->users); + } + + pr_debug("Closing nbd_dev %s. Active users = %u\n", + disk->disk_name, + atomic_read(_dev->users.refcount) - 1); +} + static const struct block_device_operations nbd_fops = { .owner =THIS_MODULE, .ioctl =nbd_ioctl, .compat_ioctl = nbd_ioctl, + .open = nbd_open, + .release = nbd_release }; + static void nbd_work_func(struct work_struct *ws_nbd) { struct nbd_device *nbd_dev = container_of(ws_nbd, struct nbd_device, @@ -1098,6 +1148,7 @@ static int __init nbd_init(void) disk->first_minor = i << part_shift; disk->fops = _fops; disk->private_data = _dev[i]; + kref_init(_dev[i].users); sprintf(disk->disk_name, "nbd%d", i); nbd_reset(_dev[i]); add_disk(disk); -- 1.7.9.5
[PATCH 3/4] make nbd device wait for its users.
When a timeout occurs or a recv fails, then instead of abruplty killing nbd block device wait for it's users to finish. This is more required when filesystem(s) like ext2 or ext3 don't expect their buffer heads to disappear while the filesystem is mounted. Use a kref for users using this. The device will be released for kref count of 2, not less or more. Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 51 +++ 1 file changed, 51 insertions(+) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index af86c9b..59db890 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -71,6 +71,8 @@ struct nbd_device { struct dentry *dbg_dir; #endif struct work_struct ws_nbd; + struct kref users; + struct completion user_completion; }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -674,6 +676,7 @@ static void nbd_reset(struct nbd_device *nbd) nbd->flags = 0; nbd->xmit_timeout = 0; INIT_WORK(>ws_nbd, nbd_work_func); + init_completion(>user_completion); queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue); del_timer_sync(>timeout_timer); } @@ -807,6 +810,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, kthread_stop(thread); sock_shutdown(nbd); + wait_for_completion(>user_completion); mutex_lock(>tx_lock); nbd_clear_que(nbd); kill_bdev(bdev); @@ -858,12 +862,58 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t mode, return error; } +static void nbd_kref_release(struct kref *kref_users) +{ + struct nbd_device *nbd = container_of(kref_users, struct nbd_device, + users); + pr_debug("Releasing kref [%s]\n", __FUNCTION__); + complete(>user_completion); + +} + +static int nbd_open(struct block_device *bdev, fmode_t mode) +{ + struct nbd_device *nbd_dev = bdev->bd_disk->private_data; + + kref_get(_dev->users); + pr_debug("Opening nbd_dev %s. Active users = %u\n", + bdev->bd_disk->disk_name, + atomic_read(_dev->users.refcount) - 1); + return 0; +} + +static void nbd_release(struct gendisk *disk, fmode_t mode) +{ + struct nbd_device *nbd_dev = disk->private_data; + /* + *kref_init initializes ref count to 1, so we + *we check for refcount to be 2 for a final put. + * + *kref needs to be re-initialized just here as the + *other process holding it must see the ref count as 2. + */ + kref_put(_dev->users, nbd_kref_release); + + if (atomic_read(_dev->users.refcount) == 2) { + kref_sub(_dev->users, 2, nbd_kref_release); + kref_init(_dev->users); + kref_get(_dev->users); + } + + pr_debug("Closing nbd_dev %s. Active users = %u\n", + disk->disk_name, + atomic_read(_dev->users.refcount) - 1); +} + static const struct block_device_operations nbd_fops = { .owner =THIS_MODULE, .ioctl =nbd_ioctl, .compat_ioctl = nbd_ioctl, + .open = nbd_open, + .release = nbd_release }; + static void nbd_work_func(struct work_struct *ws_nbd) { struct nbd_device *nbd_dev = container_of(ws_nbd, struct nbd_device, @@ -1098,6 +1148,7 @@ static int __init nbd_init(void) disk->first_minor = i << part_shift; disk->fops = _fops; disk->private_data = _dev[i]; + kref_init(_dev[i].users); sprintf(disk->disk_name, "nbd%d", i); nbd_reset(_dev[i]); add_disk(disk); -- 1.7.9.5
[PATCH 1/4] fix might_sleep warning on socket shutdown.
spinlocked ranges should be small and not contain calls into huge subfunctions. Fix my mistake and just get the pointer to the socket instead of doing everything with spinlock held. Reported-by: Mikulas Patocka <miku...@twibright.com> Signed-off-by: Markus Pargmann <m...@pengutronix.de> Changelog: Pranay Kr. Srivastava<pran...@gmail.com>: 1) Use spin_lock instead of irq version for sock_shutdown. 2) Use system work queue to actually trigger the shutdown of socket. This solves the issue when kernel_sendmsg is currently blocked while a timeout occurs. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 62 +-- 1 file changed, 35 insertions(+), 27 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 08afbc7..a5dab48 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -39,6 +39,7 @@ #include #include +#include struct nbd_device { u32 flags; @@ -57,7 +58,7 @@ struct nbd_device { int blksize; loff_t bytesize; int xmit_timeout; - bool timedout; + atomic_t timedout; bool disconnect; /* a disconnect has been requested by user */ struct timer_list timeout_timer; @@ -69,6 +70,7 @@ struct nbd_device { #if IS_ENABLED(CONFIG_DEBUG_FS) struct dentry *dbg_dir; #endif + struct work_struct ws_nbd; }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -94,6 +96,7 @@ static int max_part; * Thanks go to Jens Axboe and Al Viro for their LKML emails explaining this! */ static DEFINE_SPINLOCK(nbd_lock); +static void nbd_work_func(struct work_struct *); static inline struct device *nbd_to_dev(struct nbd_device *nbd) { @@ -172,39 +175,31 @@ static void nbd_end_request(struct nbd_device *nbd, struct request *req) */ static void sock_shutdown(struct nbd_device *nbd) { - spin_lock_irq(>sock_lock); - - if (!nbd->sock) { - spin_unlock_irq(>sock_lock); - return; - } + struct socket *sock; - dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - sockfd_put(nbd->sock); + spin_lock(>sock_lock); + sock = nbd->sock; nbd->sock = NULL; - spin_unlock_irq(>sock_lock); + spin_unlock(>sock_lock); + + if (!sock) + return; del_timer(>timeout_timer); + dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); + kernel_sock_shutdown(sock, SHUT_RDWR); + sockfd_put(sock); } static void nbd_xmit_timeout(unsigned long arg) { struct nbd_device *nbd = (struct nbd_device *)arg; - unsigned long flags; if (list_empty(>queue_head)) return; - - spin_lock_irqsave(>sock_lock, flags); - - nbd->timedout = true; - - if (nbd->sock) - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - - spin_unlock_irqrestore(>sock_lock, flags); - + atomic_inc(>timedout); + schedule_work(>ws_nbd); + wake_up(>waiting_wq); dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down connection\n"); } @@ -592,7 +587,11 @@ static int nbd_thread_send(void *data) spin_unlock_irq(>queue_lock); /* handle request */ - nbd_handle_req(nbd, req); + if (atomic_read(>timedout)) { + req->errors++; + nbd_end_request(nbd, req); + } else + nbd_handle_req(nbd, req); } nbd->task_send = NULL; @@ -666,12 +665,13 @@ out: static void nbd_reset(struct nbd_device *nbd) { nbd->disconnect = false; - nbd->timedout = false; + atomic_set(>timedout, 0); nbd->blksize = 1024; nbd->bytesize = 0; set_capacity(nbd->disk, 0); nbd->flags = 0; nbd->xmit_timeout = 0; + INIT_WORK(>ws_nbd, nbd_work_func); queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue); del_timer_sync(>timeout_timer); } @@ -804,16 +804,16 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, nbd_dev_dbg_close(nbd); kthread_stop(thread); - mutex_lock(>tx_lock); - sock_shutdown(nbd); + mutex_lock(>tx_lock); nbd_clear_que(nbd); kill_bdev(bdev); nbd_bdev_reset(bdev); if (nbd->disconnect) /* user requested, ignore socket errors */ error = 0; - if (nbd->timedout) + + if (atomic_read(>timedout)) error = -ETIMEDOUT; nbd_reset(nbd); @@ -863,6 +863,14
[PATCH 2/4] fix various coding standard warnings
1 )nbd: fix checkpatch trailing space warning. 2) nbd: fix checkpatch warning use linux/uaccess.h 3) nbd : fix checkpatch pointer declaration warning 4) nbd: fix checkpatch warning no newline after decleration. 5) nbd: fix checkpatch warning no newline after decleration. 6) nbd : fix checkpatch line over 80 char warning 7) nbd: fix checkpatch trailing whitespace warning. 8) nbd: fix checkpatch trailing whitespace warning. 9) nbd : fix checkpatch structure declaration braces on next line warning. 10) nbd : fix checkpatch trailing whitespace warning 11) nbd : fix checkpatch printk warning 12) nbd: fix checkpatch no extra line after decleration warning 13) nbd: fix checkpatch printk warning to pr_info 14) nbd: fix checkpatch no new line after decleration warning 15) nbd: fix checkpatch printk warning to pr_info Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 29 - 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index a5dab48..af86c9b 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -3,7 +3,7 @@ * * Note that you can not swap over this thing, yet. Seems to work but * deadlocks sometimes - you can not swap over TCP in general. - * + * * Copyright 1997-2000, 2008 Pavel Machek <pa...@ucw.cz> * Parts copyright 2001 Steven Whitehouse <st...@chygwyn.com> * @@ -35,7 +35,7 @@ #include #include -#include +#include #include #include @@ -43,7 +43,7 @@ struct nbd_device { u32 flags; - struct socket * sock; /* If == NULL, device is not ready, yet */ + struct socket *sock;/* If == NULL, device is not ready, yet */ int magic; spinlock_t queue_lock; @@ -261,6 +261,7 @@ static inline int sock_send_bvec(struct nbd_device *nbd, struct bio_vec *bvec, { int result; void *kaddr = kmap(bvec->bv_page); + result = sock_xmit(nbd, 1, kaddr + bvec->bv_offset, bvec->bv_len, flags); kunmap(bvec->bv_page); @@ -358,6 +359,7 @@ static inline int sock_recv_bvec(struct nbd_device *nbd, struct bio_vec *bvec) { int result; void *kaddr = kmap(bvec->bv_page); + result = sock_xmit(nbd, 0, kaddr + bvec->bv_offset, bvec->bv_len, MSG_WAITALL); kunmap(bvec->bv_page); @@ -600,8 +602,8 @@ static int nbd_thread_send(void *data) } /* - * We always wait for result of write, for now. It would be nice to make it optional - * in future + * We always wait for result of write, for now. It would be nice to make it + * optional in future * if ((rq_data_dir(req) == WRITE) && (nbd->flags & NBD_WRITE_NOCHK)) * { printk( "Warning: Ignoring result!\n"); nbd_end_request( req ); } */ @@ -610,7 +612,7 @@ static void nbd_request_handler(struct request_queue *q) __releases(q->queue_lock) __acquires(q->queue_lock) { struct request *req; - + while ((req = blk_fetch_request(q)) != NULL) { struct nbd_device *nbd; @@ -729,7 +731,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, nbd_send_req(nbd, ); return 0; } - + case NBD_CLEAR_SOCK: sock_shutdown(nbd); nbd_clear_que(nbd); @@ -856,8 +858,7 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t mode, return error; } -static const struct block_device_operations nbd_fops = -{ +static const struct block_device_operations nbd_fops = { .owner =THIS_MODULE, .ioctl =nbd_ioctl, .compat_ioctl = nbd_ioctl, @@ -1000,7 +1001,7 @@ static void nbd_dbg_close(void) #endif /* - * And here should be modules and kernel interface + * And here should be modules and kernel interface * (Just smiley confuses emacs :-) */ @@ -1013,7 +1014,7 @@ static int __init nbd_init(void) BUILD_BUG_ON(sizeof(struct nbd_request) != 28); if (max_part < 0) { - printk(KERN_ERR "nbd: max_part must be >= 0\n"); + pr_err("nbd: max_part must be >= 0\n"); return -EINVAL; } @@ -1044,6 +1045,7 @@ static int __init nbd_init(void) for (i = 0; i < nbds_max; i++) { struct gendisk *disk = alloc_disk(1 << part_shift); + if (!disk) goto out; nbd_dev[i].disk = disk; @@ -1074,12 +1076,13 @@ static int __init nbd_init(void) goto out; } - printk(KERN_INFO "nbd: registered device at major %d\n", NBD_MAJOR); + pr_info("nbd: registered device at major %d\n", NBD_MAJOR); nbd_dbg_init(); for (i = 0; i < nbds_max; i++) { struct
[PATCH 1/4] fix might_sleep warning on socket shutdown.
spinlocked ranges should be small and not contain calls into huge subfunctions. Fix my mistake and just get the pointer to the socket instead of doing everything with spinlock held. Reported-by: Mikulas Patocka Signed-off-by: Markus Pargmann Changelog: Pranay Kr. Srivastava: 1) Use spin_lock instead of irq version for sock_shutdown. 2) Use system work queue to actually trigger the shutdown of socket. This solves the issue when kernel_sendmsg is currently blocked while a timeout occurs. Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 62 +-- 1 file changed, 35 insertions(+), 27 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 08afbc7..a5dab48 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -39,6 +39,7 @@ #include #include +#include struct nbd_device { u32 flags; @@ -57,7 +58,7 @@ struct nbd_device { int blksize; loff_t bytesize; int xmit_timeout; - bool timedout; + atomic_t timedout; bool disconnect; /* a disconnect has been requested by user */ struct timer_list timeout_timer; @@ -69,6 +70,7 @@ struct nbd_device { #if IS_ENABLED(CONFIG_DEBUG_FS) struct dentry *dbg_dir; #endif + struct work_struct ws_nbd; }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -94,6 +96,7 @@ static int max_part; * Thanks go to Jens Axboe and Al Viro for their LKML emails explaining this! */ static DEFINE_SPINLOCK(nbd_lock); +static void nbd_work_func(struct work_struct *); static inline struct device *nbd_to_dev(struct nbd_device *nbd) { @@ -172,39 +175,31 @@ static void nbd_end_request(struct nbd_device *nbd, struct request *req) */ static void sock_shutdown(struct nbd_device *nbd) { - spin_lock_irq(>sock_lock); - - if (!nbd->sock) { - spin_unlock_irq(>sock_lock); - return; - } + struct socket *sock; - dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - sockfd_put(nbd->sock); + spin_lock(>sock_lock); + sock = nbd->sock; nbd->sock = NULL; - spin_unlock_irq(>sock_lock); + spin_unlock(>sock_lock); + + if (!sock) + return; del_timer(>timeout_timer); + dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); + kernel_sock_shutdown(sock, SHUT_RDWR); + sockfd_put(sock); } static void nbd_xmit_timeout(unsigned long arg) { struct nbd_device *nbd = (struct nbd_device *)arg; - unsigned long flags; if (list_empty(>queue_head)) return; - - spin_lock_irqsave(>sock_lock, flags); - - nbd->timedout = true; - - if (nbd->sock) - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - - spin_unlock_irqrestore(>sock_lock, flags); - + atomic_inc(>timedout); + schedule_work(>ws_nbd); + wake_up(>waiting_wq); dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down connection\n"); } @@ -592,7 +587,11 @@ static int nbd_thread_send(void *data) spin_unlock_irq(>queue_lock); /* handle request */ - nbd_handle_req(nbd, req); + if (atomic_read(>timedout)) { + req->errors++; + nbd_end_request(nbd, req); + } else + nbd_handle_req(nbd, req); } nbd->task_send = NULL; @@ -666,12 +665,13 @@ out: static void nbd_reset(struct nbd_device *nbd) { nbd->disconnect = false; - nbd->timedout = false; + atomic_set(>timedout, 0); nbd->blksize = 1024; nbd->bytesize = 0; set_capacity(nbd->disk, 0); nbd->flags = 0; nbd->xmit_timeout = 0; + INIT_WORK(>ws_nbd, nbd_work_func); queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue); del_timer_sync(>timeout_timer); } @@ -804,16 +804,16 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, nbd_dev_dbg_close(nbd); kthread_stop(thread); - mutex_lock(>tx_lock); - sock_shutdown(nbd); + mutex_lock(>tx_lock); nbd_clear_que(nbd); kill_bdev(bdev); nbd_bdev_reset(bdev); if (nbd->disconnect) /* user requested, ignore socket errors */ error = 0; - if (nbd->timedout) + + if (atomic_read(>timedout)) error = -ETIMEDOUT; nbd_reset(nbd); @@ -863,6 +863,14 @@ static const struct block_device_operations nbd_fops = .compat
[PATCH 2/4] fix various coding standard warnings
1 )nbd: fix checkpatch trailing space warning. 2) nbd: fix checkpatch warning use linux/uaccess.h 3) nbd : fix checkpatch pointer declaration warning 4) nbd: fix checkpatch warning no newline after decleration. 5) nbd: fix checkpatch warning no newline after decleration. 6) nbd : fix checkpatch line over 80 char warning 7) nbd: fix checkpatch trailing whitespace warning. 8) nbd: fix checkpatch trailing whitespace warning. 9) nbd : fix checkpatch structure declaration braces on next line warning. 10) nbd : fix checkpatch trailing whitespace warning 11) nbd : fix checkpatch printk warning 12) nbd: fix checkpatch no extra line after decleration warning 13) nbd: fix checkpatch printk warning to pr_info 14) nbd: fix checkpatch no new line after decleration warning 15) nbd: fix checkpatch printk warning to pr_info Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 29 - 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index a5dab48..af86c9b 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -3,7 +3,7 @@ * * Note that you can not swap over this thing, yet. Seems to work but * deadlocks sometimes - you can not swap over TCP in general. - * + * * Copyright 1997-2000, 2008 Pavel Machek * Parts copyright 2001 Steven Whitehouse * @@ -35,7 +35,7 @@ #include #include -#include +#include #include #include @@ -43,7 +43,7 @@ struct nbd_device { u32 flags; - struct socket * sock; /* If == NULL, device is not ready, yet */ + struct socket *sock;/* If == NULL, device is not ready, yet */ int magic; spinlock_t queue_lock; @@ -261,6 +261,7 @@ static inline int sock_send_bvec(struct nbd_device *nbd, struct bio_vec *bvec, { int result; void *kaddr = kmap(bvec->bv_page); + result = sock_xmit(nbd, 1, kaddr + bvec->bv_offset, bvec->bv_len, flags); kunmap(bvec->bv_page); @@ -358,6 +359,7 @@ static inline int sock_recv_bvec(struct nbd_device *nbd, struct bio_vec *bvec) { int result; void *kaddr = kmap(bvec->bv_page); + result = sock_xmit(nbd, 0, kaddr + bvec->bv_offset, bvec->bv_len, MSG_WAITALL); kunmap(bvec->bv_page); @@ -600,8 +602,8 @@ static int nbd_thread_send(void *data) } /* - * We always wait for result of write, for now. It would be nice to make it optional - * in future + * We always wait for result of write, for now. It would be nice to make it + * optional in future * if ((rq_data_dir(req) == WRITE) && (nbd->flags & NBD_WRITE_NOCHK)) * { printk( "Warning: Ignoring result!\n"); nbd_end_request( req ); } */ @@ -610,7 +612,7 @@ static void nbd_request_handler(struct request_queue *q) __releases(q->queue_lock) __acquires(q->queue_lock) { struct request *req; - + while ((req = blk_fetch_request(q)) != NULL) { struct nbd_device *nbd; @@ -729,7 +731,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, nbd_send_req(nbd, ); return 0; } - + case NBD_CLEAR_SOCK: sock_shutdown(nbd); nbd_clear_que(nbd); @@ -856,8 +858,7 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t mode, return error; } -static const struct block_device_operations nbd_fops = -{ +static const struct block_device_operations nbd_fops = { .owner =THIS_MODULE, .ioctl =nbd_ioctl, .compat_ioctl = nbd_ioctl, @@ -1000,7 +1001,7 @@ static void nbd_dbg_close(void) #endif /* - * And here should be modules and kernel interface + * And here should be modules and kernel interface * (Just smiley confuses emacs :-) */ @@ -1013,7 +1014,7 @@ static int __init nbd_init(void) BUILD_BUG_ON(sizeof(struct nbd_request) != 28); if (max_part < 0) { - printk(KERN_ERR "nbd: max_part must be >= 0\n"); + pr_err("nbd: max_part must be >= 0\n"); return -EINVAL; } @@ -1044,6 +1045,7 @@ static int __init nbd_init(void) for (i = 0; i < nbds_max; i++) { struct gendisk *disk = alloc_disk(1 << part_shift); + if (!disk) goto out; nbd_dev[i].disk = disk; @@ -1074,12 +1076,13 @@ static int __init nbd_init(void) goto out; } - printk(KERN_INFO "nbd: registered device at major %d\n", NBD_MAJOR); + pr_info("nbd: registered device at major %d\n", NBD_MAJOR); nbd_dbg_init(); for (i = 0; i < nbds_max; i++) { struct gendisk *disk = nbd_dev[i].disk; + nbd_dev[i].magic = NBD_MAGIC;
[PATCH 0/4]nbd: fixes for nbd
This patch series fixes the following 1) fix might_sleep warning on socket shutdown: Fix sock_shutdown to avoid calling kernel_sock_shutdown while holding spin_lock. 2)fix various coding standard warnings Make shutdown get called in a process context instead, using system_wq. 3) make nbd device wait for its users. When a timeout or error occurs then nbd driver simply kills the block device. Many filesystem(s) example ext2/ext3 don't expect their buffer heads to disappear like that. Fix this by making nbd device wait for its users. 4) use device_attr macros for sysfs attribute use DEVICE_ATTR_RO for sysfs pid attribute. Pranay Kr. Srivastava (4): fix might_sleep warning on socket shutdown. fix various coding standard warnings make nbd device wait for its users. use device_attr macros for sysfs attribute drivers/block/nbd.c | 150 +++ 1 file changed, 105 insertions(+), 45 deletions(-) -- 1.7.9.5
[PATCH 0/4]nbd: fixes for nbd
This patch series fixes the following 1) fix might_sleep warning on socket shutdown: Fix sock_shutdown to avoid calling kernel_sock_shutdown while holding spin_lock. 2)fix various coding standard warnings Make shutdown get called in a process context instead, using system_wq. 3) make nbd device wait for its users. When a timeout or error occurs then nbd driver simply kills the block device. Many filesystem(s) example ext2/ext3 don't expect their buffer heads to disappear like that. Fix this by making nbd device wait for its users. 4) use device_attr macros for sysfs attribute use DEVICE_ATTR_RO for sysfs pid attribute. Pranay Kr. Srivastava (4): fix might_sleep warning on socket shutdown. fix various coding standard warnings make nbd device wait for its users. use device_attr macros for sysfs attribute drivers/block/nbd.c | 150 +++ 1 file changed, 105 insertions(+), 45 deletions(-) -- 1.7.9.5
[PATCH 4/4] use device_attr macros for sysfs attribute
This patch changes the pid sysfs device attribute to use DEVICE_ATTR_* macro. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 59db890..d6ab9c9 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -436,10 +436,8 @@ static ssize_t pid_show(struct device *dev, return sprintf(buf, "%d\n", task_pid_nr(nbd->task_recv)); } -static struct device_attribute pid_attr = { - .attr = { .name = "pid", .mode = S_IRUGO}, - .show = pid_show, -}; + +static DEVICE_ATTR_RO(pid); static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev) { @@ -452,7 +450,7 @@ static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev) nbd->task_recv = current; - ret = device_create_file(disk_to_dev(nbd->disk), _attr); + ret = device_create_file(disk_to_dev(nbd->disk), _attr_pid); if (ret) { dev_err(disk_to_dev(nbd->disk), "device_create_file failed!\n"); @@ -475,7 +473,7 @@ static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev) nbd_size_clear(nbd, bdev); - device_remove_file(disk_to_dev(nbd->disk), _attr); + device_remove_file(disk_to_dev(nbd->disk), _attr_pid); nbd->task_recv = NULL; -- 1.7.9.5
[PATCH 4/4] use device_attr macros for sysfs attribute
This patch changes the pid sysfs device attribute to use DEVICE_ATTR_* macro. Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 59db890..d6ab9c9 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -436,10 +436,8 @@ static ssize_t pid_show(struct device *dev, return sprintf(buf, "%d\n", task_pid_nr(nbd->task_recv)); } -static struct device_attribute pid_attr = { - .attr = { .name = "pid", .mode = S_IRUGO}, - .show = pid_show, -}; + +static DEVICE_ATTR_RO(pid); static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev) { @@ -452,7 +450,7 @@ static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev) nbd->task_recv = current; - ret = device_create_file(disk_to_dev(nbd->disk), _attr); + ret = device_create_file(disk_to_dev(nbd->disk), _attr_pid); if (ret) { dev_err(disk_to_dev(nbd->disk), "device_create_file failed!\n"); @@ -475,7 +473,7 @@ static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev) nbd_size_clear(nbd, bdev); - device_remove_file(disk_to_dev(nbd->disk), _attr); + device_remove_file(disk_to_dev(nbd->disk), _attr_pid); nbd->task_recv = NULL; -- 1.7.9.5
[PATCH v4 03/18] nbd: fix checkpatch warning use linux/uaccess.h
Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 82aac42..c7ccde7 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -35,7 +35,7 @@ #include #include -#include +#include #include #include -- 2.6.2
[PATCH v4 03/18] nbd: fix checkpatch warning use linux/uaccess.h
Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 82aac42..c7ccde7 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -35,7 +35,7 @@ #include #include -#include +#include #include #include -- 2.6.2
[PATCH v4 06/18] nbd: fix checkpatch warning no newline after decleration.
Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 2192c0e..6a4dc3a 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -355,6 +355,7 @@ static inline int sock_recv_bvec(struct nbd_device *nbd, struct bio_vec *bvec) { int result; void *kaddr = kmap(bvec->bv_page); + result = sock_xmit(nbd, 0, kaddr + bvec->bv_offset, bvec->bv_len, MSG_WAITALL); kunmap(bvec->bv_page); -- 2.6.2
[PATCH v4 06/18] nbd: fix checkpatch warning no newline after decleration.
Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 2192c0e..6a4dc3a 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -355,6 +355,7 @@ static inline int sock_recv_bvec(struct nbd_device *nbd, struct bio_vec *bvec) { int result; void *kaddr = kmap(bvec->bv_page); + result = sock_xmit(nbd, 0, kaddr + bvec->bv_offset, bvec->bv_len, MSG_WAITALL); kunmap(bvec->bv_page); -- 2.6.2
[PATCH v4 04/18] nbd : fix checkpatch pointer declaration warning
Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index c7ccde7..786aaac 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -42,7 +42,7 @@ struct nbd_device { u32 flags; - struct socket * sock; /* If == NULL, device is not ready, yet */ + struct socket *sock;/* If == NULL, device is not ready, yet */ int magic; spinlock_t queue_lock; -- 2.6.2
[PATCH v4 04/18] nbd : fix checkpatch pointer declaration warning
Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index c7ccde7..786aaac 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -42,7 +42,7 @@ struct nbd_device { u32 flags; - struct socket * sock; /* If == NULL, device is not ready, yet */ + struct socket *sock;/* If == NULL, device is not ready, yet */ int magic; spinlock_t queue_lock; -- 2.6.2
[PATCH v4 09/18] nbd: fix checkpatch trailing whitespace warning.
Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 224b44eb..2f1e5d0 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -627,7 +627,7 @@ static void nbd_request_handler(struct request_queue *q) __releases(q->queue_lock) __acquires(q->queue_lock) { struct request *req; - + while ((req = blk_fetch_request(q)) != NULL) { struct nbd_device *nbd; -- 2.6.2
[PATCH v4 09/18] nbd: fix checkpatch trailing whitespace warning.
Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 224b44eb..2f1e5d0 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -627,7 +627,7 @@ static void nbd_request_handler(struct request_queue *q) __releases(q->queue_lock) __acquires(q->queue_lock) { struct request *req; - + while ((req = blk_fetch_request(q)) != NULL) { struct nbd_device *nbd; -- 2.6.2
[PATCH v4 13/18] nbd : fix checkpatch printk warning
Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 4fd3016..690e734 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1018,7 +1018,7 @@ static int __init nbd_init(void) BUILD_BUG_ON(sizeof(struct nbd_request) != 28); if (max_part < 0) { - printk(KERN_ERR "nbd: max_part must be >= 0\n"); + pr_err("nbd: max_part must be >= 0\n"); return -EINVAL; } -- 2.6.2
[PATCH v4 10/18] nbd: fix checkpatch trailing whitespace warning.
Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 2f1e5d0..0bc73dd 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -745,7 +745,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, nbd_send_req(nbd, ); return 0; } - + case NBD_CLEAR_SOCK: sock_shutdown(nbd); nbd_clear_que(nbd); -- 2.6.2
[PATCH v4 13/18] nbd : fix checkpatch printk warning
Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 4fd3016..690e734 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1018,7 +1018,7 @@ static int __init nbd_init(void) BUILD_BUG_ON(sizeof(struct nbd_request) != 28); if (max_part < 0) { - printk(KERN_ERR "nbd: max_part must be >= 0\n"); + pr_err("nbd: max_part must be >= 0\n"); return -EINVAL; } -- 2.6.2
[PATCH v4 10/18] nbd: fix checkpatch trailing whitespace warning.
Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 2f1e5d0..0bc73dd 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -745,7 +745,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, nbd_send_req(nbd, ); return 0; } - + case NBD_CLEAR_SOCK: sock_shutdown(nbd); nbd_clear_que(nbd); -- 2.6.2
[PATCH v4 16/18] nbd: fix checkpatch no new line after decleration warning
Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 9ce350b..e308f8b 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1086,6 +1086,7 @@ static int __init nbd_init(void) for (i = 0; i < nbds_max; i++) { struct gendisk *disk = nbd_dev[i].disk; + nbd_dev[i].magic = NBD_MAGIC; INIT_LIST_HEAD(_dev[i].waiting_queue); spin_lock_init(_dev[i].queue_lock); -- 2.6.2
[PATCH v4 16/18] nbd: fix checkpatch no new line after decleration warning
Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 9ce350b..e308f8b 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1086,6 +1086,7 @@ static int __init nbd_init(void) for (i = 0; i < nbds_max; i++) { struct gendisk *disk = nbd_dev[i].disk; + nbd_dev[i].magic = NBD_MAGIC; INIT_LIST_HEAD(_dev[i].waiting_queue); spin_lock_init(_dev[i].queue_lock); -- 2.6.2
[PATCH v4 15/18] nbd: fix checkpatch printk warning to pr_info
Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 6633ab2..9ce350b 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1080,7 +1080,7 @@ static int __init nbd_init(void) goto out; } - printk(KERN_INFO "nbd: registered device at major %d\n", NBD_MAJOR); + pr_info("nbd: registered device at major %d\n", NBD_MAJOR); nbd_dbg_init(); -- 2.6.2
[PATCH v4 15/18] nbd: fix checkpatch printk warning to pr_info
Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 6633ab2..9ce350b 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1080,7 +1080,7 @@ static int __init nbd_init(void) goto out; } - printk(KERN_INFO "nbd: registered device at major %d\n", NBD_MAJOR); + pr_info("nbd: registered device at major %d\n", NBD_MAJOR); nbd_dbg_init(); -- 2.6.2
[PATCH v4 17/18] nbd: fix checkpatch printk warning to pr_info
Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index e308f8b..482a3c0 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1134,7 +1134,7 @@ static void __exit nbd_cleanup(void) } unregister_blkdev(NBD_MAJOR, "nbd"); kfree(nbd_dev); - printk(KERN_INFO "nbd: unregistered device at major %d\n", NBD_MAJOR); + pr_info("nbd: unregistered device at major %d\n", NBD_MAJOR); } module_init(nbd_init); -- 2.6.2
[PATCH v4 17/18] nbd: fix checkpatch printk warning to pr_info
Signed-off-by: Pranay Kr. Srivastava --- drivers/block/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index e308f8b..482a3c0 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1134,7 +1134,7 @@ static void __exit nbd_cleanup(void) } unregister_blkdev(NBD_MAJOR, "nbd"); kfree(nbd_dev); - printk(KERN_INFO "nbd: unregistered device at major %d\n", NBD_MAJOR); + pr_info("nbd: unregistered device at major %d\n", NBD_MAJOR); } module_init(nbd_init); -- 2.6.2
[PATCH v4 18/18] make nbd device wait for its users in case of timeout
When a timeout occurs or a recv fails, then instead of abruplty killing nbd block device wait for it's users to finish. This is more required when filesystem(s) like ext2 or ext3 don't expect their buffer heads to disappear while the filesystem is mounted. The change is described below: a) Add a users count to nbd_device structure. b) Add a bit flag to nbd_device structure of unsigned long. If the current user count is not 1 then make nbd-client wait for the in_use bit to be cleared. Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 40 include/uapi/linux/nbd.h | 1 + 2 files changed, 41 insertions(+) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 482a3c0..9b024d8 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -59,6 +59,7 @@ struct nbd_device { int xmit_timeout; atomic_t timedout; bool disconnect; /* a disconnect has been requested by user */ + u32 users; struct timer_list timeout_timer; /* protects initialization and shutdown of the socket */ @@ -69,6 +70,7 @@ struct nbd_device { #if IS_ENABLED(CONFIG_DEBUG_FS) struct dentry *dbg_dir; #endif + unsigned long bflags; /* word size bit flags for use. */ }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -822,6 +824,15 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, sock_shutdown(nbd); mutex_lock(>tx_lock); nbd_clear_que(nbd); + /* +* Wait for any users currently using +* this block device. +*/ + mutex_unlock(>tx_lock); + pr_info("Waiting for users to release device %s ...\n", + bdev->bd_disk->disk_name); + wait_on_bit(>bflags, NBD_BFLAG_INUSE_BIT, TASK_INTERRUPTIBLE); + mutex_lock(>tx_lock); kill_bdev(bdev); nbd_bdev_reset(bdev); @@ -870,10 +881,39 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t mode, return error; } +static int nbd_open(struct block_device *bdev, fmode_t mode) +{ + struct nbd_device *nbd_dev = bdev->bd_disk->private_data; + nbd_dev->users++; + pr_debug("Opening nbd_dev %s. Active users = %u\n", + bdev->bd_disk->disk_name, nbd_dev->users); + if (nbd_dev->users > 1) + { + set_bit(NBD_BFLAG_INUSE_BIT, _dev->bflags); + } + return 0; +} + +static void nbd_release(struct gendisk *disk, fmode_t mode) +{ + struct nbd_device *nbd_dev = disk->private_data; + nbd_dev->users--; + pr_debug("Closing nbd_dev %s. Active users = %u\n", + disk->disk_name, nbd_dev->users); + if (nbd_dev->users == 1) + { + clear_bit(NBD_BFLAG_INUSE_BIT, _dev->bflags); + smp_mb(); + wake_up_bit(_dev->bflags, NBD_BFLAG_INUSE_BIT); + } +} + static const struct block_device_operations nbd_fops = { .owner =THIS_MODULE, .ioctl =nbd_ioctl, .compat_ioctl = nbd_ioctl, + .open = nbd_open, + .release = nbd_release }; #if IS_ENABLED(CONFIG_DEBUG_FS) diff --git a/include/uapi/linux/nbd.h b/include/uapi/linux/nbd.h index e08e413..8f3d3f0 100644 --- a/include/uapi/linux/nbd.h +++ b/include/uapi/linux/nbd.h @@ -44,6 +44,7 @@ enum { /* there is a gap here to match userspace */ #define NBD_FLAG_SEND_TRIM(1 << 5) /* send trim/discard */ +#define NBD_BFLAG_INUSE_BIT(1) /* bit number for bflags */ /* userspace doesn't need the nbd_device structure */ /* These are sent over the network in the request/reply magic fields */ -- 2.6.2
[PATCH v4 14/18] nbd: fix checkpatch no extra line after decleration warning
Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> --- drivers/block/nbd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 690e734..6633ab2 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -1049,6 +1049,7 @@ static int __init nbd_init(void) for (i = 0; i < nbds_max; i++) { struct gendisk *disk = alloc_disk(1 << part_shift); + if (!disk) goto out; nbd_dev[i].disk = disk; -- 2.6.2