Re: [jet.c...@intel.com: [bio] kernel BUG at drivers/block/virtio_blk.c:166!]
Hi, On 27.05.2014 13:24, Maurizio Lombardi wrote: On Tue, May 27, 2014 at 10:43:59AM +0200, Maurizio Lombardi wrote: But now I'm suspicious of this part of commit 3979ef4dcf: failed: bvec-bv_page = NULL; bvec-bv_len = 0; bvec-bv_offset = 0; bio-bi_vcnt--; blk_recount_segments(q, bio); return 0; Is decreasing bi_vcnt sufficient to guarantee that blk_recount_segments() recalculates the correct number of physical segments? Looking at the __blk_recalc_rq_segments() it appears it may not be the case. The question is how can we restore the correct number of physical segments in case of failure without breaking anything... If my hypothesis is correct, the following patch should trigger a kernel panic, Jet Chen, can you try it and let me know whether the BUG_ON is hit or not? I was also able to reproduce this bug just as reported by Jet Chen, and now I think I've found out a solution. I'll send out a patch. Regards, Dongsu diff --git a/block/bio.c b/block/bio.c index 0443694..763868f 100644 --- a/block/bio.c +++ b/block/bio.c @@ -701,6 +701,7 @@ static int __bio_add_page(struct request_queue *q, struct bio *bio, struct page unsigned int max_sectors) { int retried_segments = 0; + unsigned int phys_segments_orig; struct bio_vec *bvec; /* @@ -751,6 +752,9 @@ static int __bio_add_page(struct request_queue *q, struct bio *bio, struct page if (bio-bi_vcnt = bio-bi_max_vecs) return 0; + blk_recount_segments(q, bio); + phys_segments_orig = bio-bi_phys_segments; + /* * setup the new entry, we might clear it again later if we * cannot add the page @@ -811,6 +815,7 @@ static int __bio_add_page(struct request_queue *q, struct bio *bio, struct page bvec-bv_offset = 0; bio-bi_vcnt--; blk_recount_segments(q, bio); + BUG_ON(phys_segments_orig != bio-bi_phys_segments); return 0; } Regards, Maurizio Lombardi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] bio: decrease bi_iter.bi_size by len in the fail path
From: Dongsu Park dongsu.p...@profitbricks.com Commit 3979ef4dcf3d1de55a560a3a4016c30a835df44d (bio-modify- __bio_add_page-to-accept-pages-that-dont-start-a-new-segment-v3) introduced a regression as reported by Jet Chen. That results in a kernel BUG at drivers/block/virtio_blk.c:166. To fix that, bi_iter.bi_size must be decreased by len, before recounting the number of physical segments. Tested on with kernel 3.15.0-rc7-next-20140527 on qemu guest, by running xfstests/ext4/271. Cc: Jens Axboe ax...@kernel.dk Cc: Jet Chen jet.c...@intel.com Cc: Maurizio Lombardi mlomb...@redhat.com Signed-off-by: Dongsu Park dongsu.p...@profitbricks.com --- block/bio.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/bio.c b/block/bio.c index 0443694ccbb4..67d7cba1e5fd 100644 --- a/block/bio.c +++ b/block/bio.c @@ -810,6 +810,7 @@ static int __bio_add_page(struct request_queue *q, struct bio *bio, struct page bvec-bv_len = 0; bvec-bv_offset = 0; bio-bi_vcnt--; + bio-bi_iter.bi_size -= len; blk_recount_segments(q, bio); return 0; } -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] Tux3 for review
Hi, On 19.05.2014 17:55, Daniel Phillips wrote: > On 05/18/2014 04:55 PM, Dave Chinner wrote: > >On Fri, May 16, 2014 at 05:50:59PM -0700, Daniel Phillips wrote: > >>We would like to offer Tux3 for review for mainline merge. We have > >>prepared a new repository suitable for pulling: > >> > >>https://git.kernel.org/cgit/linux/kernel/git/daniel/linux-tux3.git/ First of all, thank you for trying to merge it to mainline. Maybe I cannot say the code is clean enough, but basically the filesystem seems to work at least. > >Then there's all the writeback hacks. You've simply copy-n-pasted > >most of fs-writeback.c, including duplicating structures like struct > >wb_writeback_work and then hacked in crap (kallsyms lookups!) to be > >able to access core structures from kernel module context > >(tux3_setup_writeback(), I'm looking at you). > This is intentional. The files named "*_hack" were kept as close as > possible to the original core code to clarify exactly where core > needs to change in order to remove our workarounds. If you think we > should pretty up that code then we will happily do it. Or maybe we > can hammer out acceptable core patches right now, and include those > with our merge proposal. That would make us even happier. We hate > those hacks as much as you do. Looking up kallsyms is not only hacky, but also making the filesystem unable to be mounted at all, when CONFIG_KALLSYMS_ALL is not defined. I'll send out patches to fix that separately to tux3 mailing list. Regards, Dongsu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] Tux3 for review
Hi, On 19.05.2014 17:55, Daniel Phillips wrote: On 05/18/2014 04:55 PM, Dave Chinner wrote: On Fri, May 16, 2014 at 05:50:59PM -0700, Daniel Phillips wrote: We would like to offer Tux3 for review for mainline merge. We have prepared a new repository suitable for pulling: https://git.kernel.org/cgit/linux/kernel/git/daniel/linux-tux3.git/ First of all, thank you for trying to merge it to mainline. Maybe I cannot say the code is clean enough, but basically the filesystem seems to work at least. Then there's all the writeback hacks. You've simply copy-n-pasted most of fs-writeback.c, including duplicating structures like struct wb_writeback_work and then hacked in crap (kallsyms lookups!) to be able to access core structures from kernel module context (tux3_setup_writeback(), I'm looking at you). This is intentional. The files named *_hack were kept as close as possible to the original core code to clarify exactly where core needs to change in order to remove our workarounds. If you think we should pretty up that code then we will happily do it. Or maybe we can hammer out acceptable core patches right now, and include those with our merge proposal. That would make us even happier. We hate those hacks as much as you do. Looking up kallsyms is not only hacky, but also making the filesystem unable to be mounted at all, when CONFIG_KALLSYMS_ALL is not defined. I'll send out patches to fix that separately to tux3 mailing list. Regards, Dongsu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 3/10] ext4: Add support FALLOC_FL_COLLAPSE_RANGE for fallocate
Hi Namjae, see below: On 19.02.2014 01:38, Namjae Jeon wrote: > This patch implements fallocate's FALLOC_FL_COLLAPSE_RANGE for Ext4. .. > + /* > + * Don't start shifting extents until we make sure the hole is big > + * enough to accomodate the shift. > + */ > + path = ext4_ext_find_extent(inode, start - 1, NULL, 0); > + depth = path->p_depth; > + extent = path[depth].p_ext; > + ex_start = extent->ee_block; > + ex_end = extent->ee_block + ext4_ext_get_actual_len(extent); > + ext4_ext_drop_refs(path); > + kfree(path); > + > + if ((start == ex_start && shift > ex_start) || > + (shift > start - ex_end)) { This line causes a compile error. So the fix would be like that: - (shift > start - ex_end)) { + (shift > start - ex_end)) > + return -EINVAL; > + > + /* Its safe to start updating extents */ .. Apart from that, the whole patchset seems to work without error, both on xfs and on ext4. Of course I had to remove Lukas' patches before testing collapse-range. So it would be excellent if two patch series could get somehow merged. Tested-by: Dongsu Park Regards, Dongsu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 3/10] ext4: Add support FALLOC_FL_COLLAPSE_RANGE for fallocate
Hi Namjae, see below: On 19.02.2014 01:38, Namjae Jeon wrote: This patch implements fallocate's FALLOC_FL_COLLAPSE_RANGE for Ext4. ...snip... + /* + * Don't start shifting extents until we make sure the hole is big + * enough to accomodate the shift. + */ + path = ext4_ext_find_extent(inode, start - 1, NULL, 0); + depth = path-p_depth; + extent = path[depth].p_ext; + ex_start = extent-ee_block; + ex_end = extent-ee_block + ext4_ext_get_actual_len(extent); + ext4_ext_drop_refs(path); + kfree(path); + + if ((start == ex_start shift ex_start) || + (shift start - ex_end)) { This line causes a compile error. So the fix would be like that: - (shift start - ex_end)) { + (shift start - ex_end)) + return -EINVAL; + + /* Its safe to start updating extents */ ...snip... Apart from that, the whole patchset seems to work without error, both on xfs and on ext4. Of course I had to remove Lukas' patches before testing collapse-range. So it would be excellent if two patch series could get somehow merged. Tested-by: Dongsu Park dongsu.p...@profitbricks.com Regards, Dongsu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] perf: define LIBLOCKDEP_LIBS to fix linker errors
EXTLIBS has to include -llockdep, so that perf can be linked against liblockdep properly by gcc 4.6 or newer. That can be done by setting LIBLOCKDEP_LIBS to "-llockdep", as it's already included in EXTLIBS. Without this fix, building perf will fail with the following errors: builtin-sched.o: In function `thread_func': /linux/tools/perf/builtin-sched.c:471: undefined reference to `liblockdep_set_thread' builtin-sched.o: In function `liblockdep_pthread_mutex_lock': /linux/tools/perf/../lib/lockdep/include/liblockdep/mutex.h:41: undefined reference to `lock_acquire' builtin-sched.o: In function `liblockdep_pthread_mutex_unlock': /linux/tools/perf/../lib/lockdep/include/liblockdep/mutex.h:48: undefined reference to `lock_release' Signed-off-by: Dongsu Park Cc: Sasha Levin --- tools/perf/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/perf/Makefile b/tools/perf/Makefile index 785cd51..8d5c9eb 100644 --- a/tools/perf/Makefile +++ b/tools/perf/Makefile @@ -576,8 +576,9 @@ ifndef NO_LIBLOCKDEP # for linking with liblockdep library, run like: # make LIBLOCKDEP=1 ifeq ($(LOCKDEP),1) + LIBLOCKDEP_LIBS := -llockdep LIBLOCKDEP_CFLAGS := -I../lib/lockdep/include -D__USE_LIBLOCKDEP - LIBLOCKDEP_LDFLAGS := -L../lib/lockdep/ -llockdep + LIBLOCKDEP_LDFLAGS := -L../lib/lockdep/ $(LIBLOCKDEP_LIBS) endif FLAGS_LIBLOCKDEP=$(LIBLOCKDEP_CFLAGS) $(ALL_CFLAGS) $(LIBLOCKDEP_LDFLAGS) $(ALL_LDFLAGS) $(EXTLIBS) -- 1.8.0.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] perf: define LIBLOCKDEP_LIBS to fix linker errors
EXTLIBS has to include -llockdep, so that perf can be linked against liblockdep properly by gcc 4.6 or newer. That can be done by setting LIBLOCKDEP_LIBS to -llockdep, as it's already included in EXTLIBS. Without this fix, building perf will fail with the following errors: builtin-sched.o: In function `thread_func': /linux/tools/perf/builtin-sched.c:471: undefined reference to `liblockdep_set_thread' builtin-sched.o: In function `liblockdep_pthread_mutex_lock': /linux/tools/perf/../lib/lockdep/include/liblockdep/mutex.h:41: undefined reference to `lock_acquire' builtin-sched.o: In function `liblockdep_pthread_mutex_unlock': /linux/tools/perf/../lib/lockdep/include/liblockdep/mutex.h:48: undefined reference to `lock_release' Signed-off-by: Dongsu Park dongsu.p...@profitbricks.com Cc: Sasha Levin sasha.le...@oracle.com --- tools/perf/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/perf/Makefile b/tools/perf/Makefile index 785cd51..8d5c9eb 100644 --- a/tools/perf/Makefile +++ b/tools/perf/Makefile @@ -576,8 +576,9 @@ ifndef NO_LIBLOCKDEP # for linking with liblockdep library, run like: # make LIBLOCKDEP=1 ifeq ($(LOCKDEP),1) + LIBLOCKDEP_LIBS := -llockdep LIBLOCKDEP_CFLAGS := -I../lib/lockdep/include -D__USE_LIBLOCKDEP - LIBLOCKDEP_LDFLAGS := -L../lib/lockdep/ -llockdep + LIBLOCKDEP_LDFLAGS := -L../lib/lockdep/ $(LIBLOCKDEP_LIBS) endif FLAGS_LIBLOCKDEP=$(LIBLOCKDEP_CFLAGS) $(ALL_CFLAGS) $(LIBLOCKDEP_LDFLAGS) $(ALL_LDFLAGS) $(EXTLIBS) -- 1.8.0.3 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/5] ib_srp: free memory correctly in srp_free_iu()
From: Dongsu Park As a potential fix for a race condition in srp_free_iu(), hold a mutex in srp_free_target_ib() before calling srp_free_iu(). In addition, also clear rx/tx ring after freeing memory. Both rx_ring[] and tx_ring[] should be reinitialized to NULL, to prevent other tasks from accessing the freed memory. Signed-off-by: Dongsu Park --- drivers/infiniband/ulp/srp/ib_srp.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index 7ae5a00..a0d0ca2 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -291,10 +291,16 @@ static void srp_free_target_ib(struct srp_target_port *target) ib_destroy_cq(target->send_cq); ib_destroy_cq(target->recv_cq); - for (i = 0; i < SRP_RQ_SIZE; ++i) + mutex_lock(>mutex); + for (i = 0; i < SRP_RQ_SIZE; ++i) { srp_free_iu(target->srp_host, target->rx_ring[i]); - for (i = 0; i < SRP_SQ_SIZE; ++i) + target->rx_ring[i] = NULL; + } + for (i = 0; i < SRP_SQ_SIZE; ++i) { srp_free_iu(target->srp_host, target->tx_ring[i]); + target->tx_ring[i] = NULL; + } + mutex_unlock(>mutex); } static void srp_path_rec_completion(int status, -- 1.7.11.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/5] ib_srp: removed superfluous warning in send timeout case
From: Dongsu Park Signed-off-By: Sebastian Riemer --- drivers/infiniband/ulp/srp/ib_srp.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index a0d0ca2..1b274484 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -534,7 +534,6 @@ static void srp_wait_last_send_wqe(struct srp_target_port *target) msleep(20); } - WARN_ON(!target->last_send_wqe); } static void srp_disconnect_target(struct srp_target_port *target) -- 1.7.11.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/5] ib_srp: fix an error accessing invalid memory in rport_dev_loss_timedout
From: Bart Van Assche In rport_dev_loss_timedout(), rport must be obtained by accessing the member entry dev_loss_work, not fast_io_fail_work. Signed-off-By: Bart Van Assche --- drivers/scsi/scsi_transport_srp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c index 915b355..d796413 100644 --- a/drivers/scsi/scsi_transport_srp.c +++ b/drivers/scsi/scsi_transport_srp.c @@ -242,7 +242,7 @@ static void rport_fast_io_fail_timedout(struct work_struct *work) { struct srp_rport *rport = container_of(to_delayed_work(work), struct srp_rport, -fast_io_fail_work); +dev_loss_work); struct Scsi_Host *shost; struct srp_internal *i; -- 1.7.11.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/5] ib_srp: check if rport->lld_data is NULL before removing rport
From: Dongsu Park After removing rport_delete(), rport->lld_data has to be set to NULL. In addition to that, both srp_rport_delete() and rport_dev_loss_timedout() must check if rport->lld_data is NULL, before accessing to rport->lld_data or any rport's target area. Without this patch, the initiator's kernel could crash with the following call trace, especially deleting remote ports as well as IB link down cases. How to reproduce: 1. Configure 500+ vdisks on target, and get initiator connected. 2. Exchange data intensively, which works well. 3. (On initiator) delete SRP remote port occasionally, e.g. # echo "1" > /sys/class/srp_remote_ports/port-6\:1/delete And configure again the SRP target. 4. (On target) disable Infiniband interface, and enable it again. 5. Repeat 3 and 4. Then the initiator's kernel suddenly crashes. (but not always) Kernel Call Trace: BUG: unable to handle kernel paging request at 00010001 IP: [] strnlen+0x5/0x40 PGD 212fea067 PUD 2162f8067 PMD 0 Oops: [#1] SMP CPU 0 Pid: 2311, comm: kworker/0:2 Not tainted 3.2.8 #1 Supermicro H8DGU/H8DGU RIP: 0010:[] [] strnlen+0x5/0x40 Process kworker/0:2 (pid: 2311, threadinfo 880215fe2000, task 88020f2ce540) Call Trace: [] ? string+0x4c/0xe0 [] ? vsnprintf+0x1ed/0x5b0 [] ? do_srp_rport_del+0x30/0x30 [scsi_transport_srp] [] ? vscnprintf+0x9/0x20 [] ? vprintk+0xaf/0x440 [] ? next_online_pgdat+0x20/0x50 [] ? next_zone+0x30/0x40 [] ? refresh_cpu_vm_stats+0xf0/0x160 [] ? do_srp_rport_del+0x30/0x30 [scsi_transport_srp] [] ? printk+0x40/0x4a [] ? rport_dev_loss_timedout+0x2d/0xa0 [scsi_transport_srp] [] ? process_one_work+0x113/0x470 [] ? worker_thread+0x163/0x3e0 [] ? manage_workers+0x200/0x200 [] ? manage_workers+0x200/0x200 [] ? kthread+0x96/0xa0 [] ? kernel_thread_helper+0x4/0x10 [] ? kthread_worker_fn+0x180/0x180 [] ? gs_change+0x13/0x13 RIP [] strnlen+0x5/0x40 RSP CR2: 00010001 ---[ end trace d55b61cd78c54a0a ]--- IP: [] kthread_data+0x7/0x10 Oops: [#2] SMP CPU 3 Pid: 16745, comm: kworker/3:4 Tainted: G DO 3.2.8-pserver+ #51 System manufacturer System Product Name/M4A89GTD-PRO RIP: 0010:[] [] kthread_data+0x7/0x10 Process kworker/3:4 (pid: 16745, threadinfo 8801f8162000, task 88020ff91440) Call Trace: [] ? wq_worker_sleeping+0x8/0x90 [] ? __schedule+0x432/0x7e0 [] ? do_exit+0x5d4/0x8a0 [] ? printk+0x40/0x4a [] ? oops_end+0xa3/0xf0 [] ? no_context+0xfd/0x270 [] ? check_preempt_wakeup+0x155/0x1d0 [] ? do_page_fault+0x31a/0x440 [] ? select_task_rq_fair+0x432/0x9d0 [] ? cpumask_next_and+0x22/0x40 [] ? find_busiest_group+0x1f3/0xb30 [] ? page_fault+0x25/0x30 [] ? strnlen+0x5/0x40 [] ? string+0x4c/0xe0 [] ? vsnprintf+0x1ed/0x5b0 [] ? do_srp_rport_del+0x30/0x30 [scsi_transport_srp] [] ? vscnprintf+0x9/0x20 [] ? vprintk+0xaf/0x440 [] ? ns_to_timeval+0x9/0x40 [] ? queue_delayed_work_on+0x157/0x170 [] ? do_srp_rport_del+0x30/0x30 [scsi_transport_srp] [] ? printk+0x40/0x4a [] ? rport_dev_loss_timedout+0x2d/0xa0 [scsi_transport_srp] [] ? cpufreq_governor_dbs+0x4b0/0x4b0 [] ? process_one_work+0x113/0x470 [] ? worker_thread+0x163/0x3e0 [] ? manage_workers+0x200/0x200 [] ? manage_workers+0x200/0x200 [] ? kthread+0x96/0xa0 [] ? kernel_thread_helper+0x4/0x10 [] ? kthread_worker_fn+0x180/0x180 [] ? gs_change+0x13/0x13 RIP [] kthread_data+0x7/0x10 RSP CR2: fff8 ---[ end trace cab7f2c38a7f7ba9 ]--- Signed-off-by: Dongsu Park --- drivers/infiniband/ulp/srp/ib_srp.c | 12 +++- drivers/scsi/scsi_transport_srp.c | 6 ++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index 1b274484..ba7bbfd 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -647,9 +647,19 @@ static void srp_remove_work(struct work_struct *work) static void srp_rport_delete(struct srp_rport *rport) { - struct srp_target_port *target = rport->lld_data; + struct srp_target_port *target; + + if (!rport->lld_data) { + pr_warn("skipping srp_rport_delete. rport->lld_data=%p\n", + rport->lld_data); + return; + } + + target = rport->lld_data; srp_queue_remove_work(target); + + rport->lld_data = NULL; } /** diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c index af3cb56..915b355 100644 --- a/drivers/scsi/scsi_transport_srp.c +++ b/drivers/scsi/scsi_transport_srp.c @@ -272,6 +272,12 @@ static void rport_dev_loss_timedout(struct work_struct *work) struct Scsi_Host *shost; struct srp_internal *i; + if (!rport->lld_data) { + pr_warn("skipping rport_delete, rport->lld_data=%p\n", + rport->lld_data); + return; + } + pr_err("SRP transport: dev_lo
[PATCH 3/5] ib_srp: hold a mutex when adding a new target port
From: Dongsu Park Unter circumstances, srp_rport_add() can make conflicts with srp_rport_delete(), dumping the call trace written below. That does not always occur. But its possible reason is adding sysfs entries for the SRP target too fast, even before the deletion hasn't finished yet. The possible solution is therefore holding a scan_mutex when calling device_add(). Example call trace: [ cut here ] WARNING: at block/genhd.c:1466 __disk_unblock_events+0x10f/0x120() Pid: 17238, comm: scsi_id Not tainted 3.2.8-pserver #1 Call Trace: [] ? warn_slowpath_common+0x7b/0xc0 [] ? __disk_unblock_events+0x10f/0x120 [] ? __blkdev_get+0x190/0x410 [] ? blkdev_get+0x310/0x310 [] ? blkdev_get+0x4b/0x310 [] ? blkdev_get+0x310/0x310 [] ? __dentry_open+0x263/0x370 [] ? path_get+0x1e/0x30 [] ? do_last+0x3e0/0x800 [] ? path_openat+0xdb/0x400 [] ? do_filp_open+0x4d/0xc0 [] ? alloc_fd+0x43/0x130 [] ? do_sys_open+0x105/0x1e0 [] ? system_call_fastpath+0x16/0x1b ---[ end trace 4edc2747f936431c ]--- [ cut here ] WARNING: at fs/sysfs/inode.c:323 sysfs_hash_and_remove+0xa4/0xb0() Hardware name: H8DGU sysfs: can not remove 'bsg', no directory Pid: 15816, comm: kworker/4:8 Tainted: GW3.2.8 #1 Call Trace: [] ? warn_slowpath_common+0x7b/0xc0 [] ? warn_slowpath_fmt+0x45/0x50 [] ? sysfs_hash_and_remove+0xa4/0xb0 [] ? bsg_unregister_queue+0x3f/0x80 [] ? __scsi_remove_device+0x99/0xc0 [scsi_mod] [] ? scsi_forget_host+0x64/0x70 [scsi_mod] [] ? scsi_remove_host+0x61/0x100 [scsi_mod] [] ? srp_remove_work+0x137/0x1c0 [ib_srp] [] ? srp_free_req_data+0xd0/0xd0 [ib_srp] [] ? process_one_work+0x113/0x470 [] ? manage_workers+0x180/0x200 [] ? worker_thread+0x163/0x3e0 [] ? manage_workers+0x200/0x200 [] ? manage_workers+0x200/0x200 [] ? kthread+0x96/0xa0 [] ? kernel_thread_helper+0x4/0x10 [] ? kthread_worker_fn+0x180/0x180 [] ? gs_change+0x13/0x13 ---[ end trace 4edc2747f936431d ]--- [ cut here ] Signed-off-by: Dongsu Park --- drivers/scsi/scsi_transport_srp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c index 7f17686..af3cb56 100644 --- a/drivers/scsi/scsi_transport_srp.c +++ b/drivers/scsi/scsi_transport_srp.c @@ -407,12 +407,15 @@ struct srp_rport *srp_rport_add(struct Scsi_Host *shost, transport_setup_device(>dev); + mutex_lock(>scan_mutex); ret = device_add(>dev); if (ret) { + mutex_unlock(>scan_mutex); transport_destroy_device(>dev); put_device(>dev); return ERR_PTR(ret); } + mutex_unlock(>scan_mutex); if (shost->active_mode & MODE_TARGET && ids->roles == SRP_RPORT_ROLE_INITIATOR) { -- 1.7.11.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/5] Fix bugs in ib_srp patches for H.A. purposes
From: Dongsu Park Hi Bart, This patchset aims at fixing bugs that have been discovered in our own SRP test environment so far. These patches are based on your patchset v4, "Make ib_srp better suited for H.A. purposes",(09 Aug 2012). The 5th patch, "fix an error accessing invalid memory in rport_dev_loss_timedout" is including your suggestion (30 Aug 2012). You can also pull the following git repo to get the patches. git://github.com/advance38/linux.git srp-ha Our test setup consists of two systems. Kernel 3.2.15 with SCST v4193 on the target, and Kernel 3.2.8 with ib_srp-ha on the initiator. Although I rebased the patches again onto 3.6-rc3, I suppose there will be no significant differences. All of the known critical issues seem to have been resolved according to our internal tests in the last weeks. Thanks, Dongsu Park (3): ib_srp: free memory correctly in srp_free_iu() ib_srp: hold a mutex when adding a new target port ib_srp: check if rport->lld_data is NULL before removing rport Sebastian Riemer (1): ib_srp: removed superfluous warning in send timeout case Bart Van Assche (1): ib_srp: fix an error accessing invalid memory in rport_dev_loss_timedout drivers/infiniband/ulp/srp/ib_srp.c | 23 +++ drivers/scsi/scsi_transport_srp.c | 11 ++- 2 files changed, 29 insertions(+), 5 deletions(-) -- 1.7.11.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/5] Fix bugs in ib_srp patches for H.A. purposes
From: Dongsu Park dongsu.p...@profitbricks.com Hi Bart, This patchset aims at fixing bugs that have been discovered in our own SRP test environment so far. These patches are based on your patchset v4, Make ib_srp better suited for H.A. purposes,(09 Aug 2012). The 5th patch, fix an error accessing invalid memory in rport_dev_loss_timedout is including your suggestion (30 Aug 2012). You can also pull the following git repo to get the patches. git://github.com/advance38/linux.git srp-ha Our test setup consists of two systems. Kernel 3.2.15 with SCST v4193 on the target, and Kernel 3.2.8 with ib_srp-ha on the initiator. Although I rebased the patches again onto 3.6-rc3, I suppose there will be no significant differences. All of the known critical issues seem to have been resolved according to our internal tests in the last weeks. Thanks, Dongsu Park (3): ib_srp: free memory correctly in srp_free_iu() ib_srp: hold a mutex when adding a new target port ib_srp: check if rport-lld_data is NULL before removing rport Sebastian Riemer (1): ib_srp: removed superfluous warning in send timeout case Bart Van Assche (1): ib_srp: fix an error accessing invalid memory in rport_dev_loss_timedout drivers/infiniband/ulp/srp/ib_srp.c | 23 +++ drivers/scsi/scsi_transport_srp.c | 11 ++- 2 files changed, 29 insertions(+), 5 deletions(-) -- 1.7.11.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/5] ib_srp: hold a mutex when adding a new target port
From: Dongsu Park dongsu.p...@profitbricks.com Unter circumstances, srp_rport_add() can make conflicts with srp_rport_delete(), dumping the call trace written below. That does not always occur. But its possible reason is adding sysfs entries for the SRP target too fast, even before the deletion hasn't finished yet. The possible solution is therefore holding a scan_mutex when calling device_add(). Example call trace: [ cut here ] WARNING: at block/genhd.c:1466 __disk_unblock_events+0x10f/0x120() Pid: 17238, comm: scsi_id Not tainted 3.2.8-pserver #1 Call Trace: [81048dbb] ? warn_slowpath_common+0x7b/0xc0 [813879bf] ? __disk_unblock_events+0x10f/0x120 [81162b30] ? __blkdev_get+0x190/0x410 [811630c0] ? blkdev_get+0x310/0x310 [81162dfb] ? blkdev_get+0x4b/0x310 [811630c0] ? blkdev_get+0x310/0x310 [8112d513] ? __dentry_open+0x263/0x370 [8113a0fe] ? path_get+0x1e/0x30 [8113b4a0] ? do_last+0x3e0/0x800 [8113c21b] ? path_openat+0xdb/0x400 [8113c66d] ? do_filp_open+0x4d/0xc0 [81148c13] ? alloc_fd+0x43/0x130 [8112d915] ? do_sys_open+0x105/0x1e0 [8165d512] ? system_call_fastpath+0x16/0x1b ---[ end trace 4edc2747f936431c ]--- [ cut here ] WARNING: at fs/sysfs/inode.c:323 sysfs_hash_and_remove+0xa4/0xb0() Hardware name: H8DGU sysfs: can not remove 'bsg', no directory Pid: 15816, comm: kworker/4:8 Tainted: GW3.2.8 #1 Call Trace: [81048dbb] ? warn_slowpath_common+0x7b/0xc0 [81048eb5] ? warn_slowpath_fmt+0x45/0x50 [8119a854] ? sysfs_hash_and_remove+0xa4/0xb0 [8138aaaf] ? bsg_unregister_queue+0x3f/0x80 [a000eda9] ? __scsi_remove_device+0x99/0xc0 [scsi_mod] [a000b3b4] ? scsi_forget_host+0x64/0x70 [scsi_mod] [a00035b1] ? scsi_remove_host+0x61/0x100 [scsi_mod] [a0643097] ? srp_remove_work+0x137/0x1c0 [ib_srp] [a0642f60] ? srp_free_req_data+0xd0/0xd0 [ib_srp] [81063383] ? process_one_work+0x113/0x470 [81065a90] ? manage_workers+0x180/0x200 [81065c73] ? worker_thread+0x163/0x3e0 [81065b10] ? manage_workers+0x200/0x200 [81065b10] ? manage_workers+0x200/0x200 [8106a126] ? kthread+0x96/0xa0 [8165f674] ? kernel_thread_helper+0x4/0x10 [8106a090] ? kthread_worker_fn+0x180/0x180 [8165f670] ? gs_change+0x13/0x13 ---[ end trace 4edc2747f936431d ]--- [ cut here ] Signed-off-by: Dongsu Park dongsu.p...@profitbricks.com --- drivers/scsi/scsi_transport_srp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c index 7f17686..af3cb56 100644 --- a/drivers/scsi/scsi_transport_srp.c +++ b/drivers/scsi/scsi_transport_srp.c @@ -407,12 +407,15 @@ struct srp_rport *srp_rport_add(struct Scsi_Host *shost, transport_setup_device(rport-dev); + mutex_lock(shost-scan_mutex); ret = device_add(rport-dev); if (ret) { + mutex_unlock(shost-scan_mutex); transport_destroy_device(rport-dev); put_device(rport-dev); return ERR_PTR(ret); } + mutex_unlock(shost-scan_mutex); if (shost-active_mode MODE_TARGET ids-roles == SRP_RPORT_ROLE_INITIATOR) { -- 1.7.11.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/5] ib_srp: check if rport-lld_data is NULL before removing rport
From: Dongsu Park dongsu.p...@profitbricks.com After removing rport_delete(), rport-lld_data has to be set to NULL. In addition to that, both srp_rport_delete() and rport_dev_loss_timedout() must check if rport-lld_data is NULL, before accessing to rport-lld_data or any rport's target area. Without this patch, the initiator's kernel could crash with the following call trace, especially deleting remote ports as well as IB link down cases. How to reproduce: 1. Configure 500+ vdisks on target, and get initiator connected. 2. Exchange data intensively, which works well. 3. (On initiator) delete SRP remote port occasionally, e.g. # echo 1 /sys/class/srp_remote_ports/port-6\:1/delete And configure again the SRP target. 4. (On target) disable Infiniband interface, and enable it again. 5. Repeat 3 and 4. Then the initiator's kernel suddenly crashes. (but not always) Kernel Call Trace: BUG: unable to handle kernel paging request at 00010001 IP: [8139ec55] strnlen+0x5/0x40 PGD 212fea067 PUD 2162f8067 PMD 0 Oops: [#1] SMP CPU 0 Pid: 2311, comm: kworker/0:2 Not tainted 3.2.8 #1 Supermicro H8DGU/H8DGU RIP: 0010:[8139ec55] [8139ec55] strnlen+0x5/0x40 Process kworker/0:2 (pid: 2311, threadinfo 880215fe2000, task 88020f2ce540) Call Trace: [813a023c] ? string+0x4c/0xe0 [813a142d] ? vsnprintf+0x1ed/0x5b0 [a0131900] ? do_srp_rport_del+0x30/0x30 [scsi_transport_srp] [813a18a9] ? vscnprintf+0x9/0x20 [81049b7f] ? vprintk+0xaf/0x440 [810f3cc0] ? next_online_pgdat+0x20/0x50 [810f3d20] ? next_zone+0x30/0x40 [810f4c60] ? refresh_cpu_vm_stats+0xf0/0x160 [a0131900] ? do_srp_rport_del+0x30/0x30 [scsi_transport_srp] [816533b6] ? printk+0x40/0x4a [a013192d] ? rport_dev_loss_timedout+0x2d/0xa0 [scsi_transport_srp] [81063383] ? process_one_work+0x113/0x470 [81065c73] ? worker_thread+0x163/0x3e0 [81065b10] ? manage_workers+0x200/0x200 [81065b10] ? manage_workers+0x200/0x200 [8106a126] ? kthread+0x96/0xa0 [8165f674] ? kernel_thread_helper+0x4/0x10 [8106a090] ? kthread_worker_fn+0x180/0x180 [8165f670] ? gs_change+0x13/0x13 RIP [8139ec55] strnlen+0x5/0x40 RSP 880215fe3c28 CR2: 00010001 ---[ end trace d55b61cd78c54a0a ]--- IP: [81069cb7] kthread_data+0x7/0x10 Oops: [#2] SMP CPU 3 Pid: 16745, comm: kworker/3:4 Tainted: G DO 3.2.8-pserver+ #51 System manufacturer System Product Name/M4A89GTD-PRO RIP: 0010:[81069cb7] [81069cb7] kthread_data+0x7/0x10 Process kworker/3:4 (pid: 16745, threadinfo 8801f8162000, task 88020ff91440) Call Trace: [81062fc8] ? wq_worker_sleeping+0x8/0x90 [81653ca2] ? __schedule+0x432/0x7e0 [8104ce54] ? do_exit+0x5d4/0x8a0 [816533b6] ? printk+0x40/0x4a [81656d13] ? oops_end+0xa3/0xf0 [8102b33d] ? no_context+0xfd/0x270 [8103dd55] ? check_preempt_wakeup+0x155/0x1d0 [8165951a] ? do_page_fault+0x31a/0x440 [810406d2] ? select_task_rq_fair+0x432/0x9d0 [81396b32] ? cpumask_next_and+0x22/0x40 [810391f3] ? find_busiest_group+0x1f3/0xb30 [81656285] ? page_fault+0x25/0x30 [8139ec55] ? strnlen+0x5/0x40 [813a023c] ? string+0x4c/0xe0 [813a142d] ? vsnprintf+0x1ed/0x5b0 [a0121900] ? do_srp_rport_del+0x30/0x30 [scsi_transport_srp] [813a18a9] ? vscnprintf+0x9/0x20 [81049b7f] ? vprintk+0xaf/0x440 [8104e429] ? ns_to_timeval+0x9/0x40 [81062f87] ? queue_delayed_work_on+0x157/0x170 [a0121900] ? do_srp_rport_del+0x30/0x30 [scsi_transport_srp] [816533b6] ? printk+0x40/0x4a [a012192d] ? rport_dev_loss_timedout+0x2d/0xa0 [scsi_transport_srp] [814f43f0] ? cpufreq_governor_dbs+0x4b0/0x4b0 [81063383] ? process_one_work+0x113/0x470 [81065c73] ? worker_thread+0x163/0x3e0 [81065b10] ? manage_workers+0x200/0x200 [81065b10] ? manage_workers+0x200/0x200 [8106a126] ? kthread+0x96/0xa0 [8165f674] ? kernel_thread_helper+0x4/0x10 [8106a090] ? kthread_worker_fn+0x180/0x180 [8165f670] ? gs_change+0x13/0x13 RIP [81069cb7] kthread_data+0x7/0x10 RSP 8801f81638d0 CR2: fff8 ---[ end trace cab7f2c38a7f7ba9 ]--- Signed-off-by: Dongsu Park dongsu.p...@profitbricks.com --- drivers/infiniband/ulp/srp/ib_srp.c | 12 +++- drivers/scsi/scsi_transport_srp.c | 6 ++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index 1b274484..ba7bbfd 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -647,9 +647,19 @@ static void srp_remove_work(struct work_struct *work) static void srp_rport_delete(struct srp_rport *rport) { - struct
[PATCH 5/5] ib_srp: fix an error accessing invalid memory in rport_dev_loss_timedout
From: Bart Van Assche bvanass...@acm.org In rport_dev_loss_timedout(), rport must be obtained by accessing the member entry dev_loss_work, not fast_io_fail_work. Signed-off-By: Bart Van Assche bvanass...@acm.org --- drivers/scsi/scsi_transport_srp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c index 915b355..d796413 100644 --- a/drivers/scsi/scsi_transport_srp.c +++ b/drivers/scsi/scsi_transport_srp.c @@ -242,7 +242,7 @@ static void rport_fast_io_fail_timedout(struct work_struct *work) { struct srp_rport *rport = container_of(to_delayed_work(work), struct srp_rport, -fast_io_fail_work); +dev_loss_work); struct Scsi_Host *shost; struct srp_internal *i; -- 1.7.11.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/5] ib_srp: removed superfluous warning in send timeout case
From: Dongsu Park dongsu.p...@profitbricks.com Signed-off-By: Sebastian Riemer sebastian.rie...@profitbricks.com --- drivers/infiniband/ulp/srp/ib_srp.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index a0d0ca2..1b274484 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -534,7 +534,6 @@ static void srp_wait_last_send_wqe(struct srp_target_port *target) msleep(20); } - WARN_ON(!target-last_send_wqe); } static void srp_disconnect_target(struct srp_target_port *target) -- 1.7.11.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/5] ib_srp: free memory correctly in srp_free_iu()
From: Dongsu Park dongsu.p...@profitbricks.com As a potential fix for a race condition in srp_free_iu(), hold a mutex in srp_free_target_ib() before calling srp_free_iu(). In addition, also clear rx/tx ring after freeing memory. Both rx_ring[] and tx_ring[] should be reinitialized to NULL, to prevent other tasks from accessing the freed memory. Signed-off-by: Dongsu Park dongsu.p...@profitbricks.com --- drivers/infiniband/ulp/srp/ib_srp.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index 7ae5a00..a0d0ca2 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -291,10 +291,16 @@ static void srp_free_target_ib(struct srp_target_port *target) ib_destroy_cq(target-send_cq); ib_destroy_cq(target-recv_cq); - for (i = 0; i SRP_RQ_SIZE; ++i) + mutex_lock(target-mutex); + for (i = 0; i SRP_RQ_SIZE; ++i) { srp_free_iu(target-srp_host, target-rx_ring[i]); - for (i = 0; i SRP_SQ_SIZE; ++i) + target-rx_ring[i] = NULL; + } + for (i = 0; i SRP_SQ_SIZE; ++i) { srp_free_iu(target-srp_host, target-tx_ring[i]); + target-tx_ring[i] = NULL; + } + mutex_unlock(target-mutex); } static void srp_path_rec_completion(int status, -- 1.7.11.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/