Re: [PATCH] blktrace: Fix potentail deadlock between delete & sysfs ops

2017-08-15 Thread Steven Rostedt
On Thu, 10 Aug 2017 13:02:33 -0400 Waiman Long wrote: > The lockdep code had reported the following unsafe locking scenario: > >CPU0CPU1 > > lock(s_active#228); >

Re: [PATCH V4 08/12] blktrace: export cgroup info in trace

2017-06-28 Thread Steven Rostedt
> --- > include/uapi/linux/blktrace_api.h | 3 + > kernel/trace/blktrace.c | 231 > ++ > 2 files changed, 161 insertions(+), 73 deletions(-) Doing a quick scan of the patch, nothing sticks out as an issue to me. Acked-by: Steven Rostedt (VMware) <rost...@goodmis.org> -- Steve

Re: [PATCH v2] blktrace: Fix potentail deadlock between delete & sysfs ops

2017-08-18 Thread Steven Rostedt
On Fri, 18 Aug 2017 16:21:46 + Bart Van Assche wrote: > Sorry Waiman but personally I thoroughly detest loops around mutex_trylock() > or > mutex_timed_lock() because such loops are usually used to paper over a problem > instead of fixing the root cause. 100% agree.

Re: [PATCH v2] blktrace: Fix potentail deadlock between delete & sysfs ops

2017-08-17 Thread Steven Rostedt
On Thu, 17 Aug 2017 18:13:20 -0400 Steven Rostedt <rost...@goodmis.org> wrote: > On Thu, 17 Aug 2017 17:27:22 -0400 > Waiman Long <long...@redhat.com> wrote: > > > > It is actually what the patch is trying to do by checking for the > > deletion flag in

Re: [PATCH v2] blktrace: Fix potentail deadlock between delete & sysfs ops

2017-08-17 Thread Steven Rostedt
On Thu, 17 Aug 2017 18:18:18 -0400 Steven Rostedt <rost...@goodmis.org> wrote: > On Thu, 17 Aug 2017 18:13:20 -0400 > Steven Rostedt <rost...@goodmis.org> wrote: > > > On Thu, 17 Aug 2017 17:27:22 -0400 > > Waiman Long <long...@redhat.com> wrote: > >

Re: [PATCH v2] blktrace: Fix potentail deadlock between delete & sysfs ops

2017-08-17 Thread Steven Rostedt
On Thu, 17 Aug 2017 20:41:10 + Bart Van Assche <bart.vanass...@wdc.com> wrote: > On Thu, 2017-08-17 at 16:30 -0400, Steven Rostedt wrote: > > On Thu, 17 Aug 2017 12:24:39 -0400 > > Waiman Long <long...@redhat.com> wrote: > > > > > On

Re: [PATCH v2] blktrace: Fix potentail deadlock between delete & sysfs ops

2017-08-17 Thread Steven Rostedt
On Thu, 17 Aug 2017 12:24:39 -0400 Waiman Long <long...@redhat.com> wrote: > On 08/17/2017 09:34 AM, Steven Rostedt wrote: > > On Wed, 16 Aug 2017 16:40:40 -0400 > > Waiman Long <long...@redhat.com> wrote: > > > >> The lockdep code had repo

Re: [PATCH v2] blktrace: Fix potentail deadlock between delete & sysfs ops

2017-08-17 Thread Steven Rostedt
On Thu, 17 Aug 2017 17:10:07 -0400 Steven Rostedt <rost...@goodmis.org> wrote: > Instead of playing games with taking the lock, the only way this race > is hit, is if the partition is being deleted and the sysfs attribute is > being read at the same time, correct? In that ca

Re: [PATCH v2] blktrace: Fix potentail deadlock between delete & sysfs ops

2017-08-17 Thread Steven Rostedt
On Thu, 17 Aug 2017 17:27:22 -0400 Waiman Long wrote: > It is actually what the patch is trying to do by checking for the > deletion flag in the mutex_trylock loop. Please note that mutex does not > guarantee FIFO ordering of lock acquisition. As a result, cpu1 may call >

Re: [PATCH v2] blktrace: Fix potentail deadlock between delete & sysfs ops

2017-08-17 Thread Steven Rostedt
On Thu, 17 Aug 2017 12:24:39 -0400 Waiman Long wrote: > >> + * sysfs file and then acquiring the bd_mutex. Deleting a block device > >> + * requires acquiring the bd_mutex and then waiting for all the sysfs > >> + * references to be gone. This can lead to deadlock if both

Re: [PATCH v2] blktrace: Fix potentail deadlock between delete & sysfs ops

2017-08-17 Thread Steven Rostedt
On Wed, 16 Aug 2017 16:40:40 -0400 Waiman Long wrote: > The lockdep code had reported the following unsafe locking scenario: > >CPU0CPU1 > > lock(s_active#228); >

Re: [PATCH v6 0/2] blktrace: Fix deadlock problem

2017-09-18 Thread Steven Rostedt
Acked-by: Steven Rostedt (VMware) <rost...@goodmis.org> for the series. Jens, feel free to take this in your tree. -- Steve On Mon, 18 Sep 2017 14:53:49 -0400 Waiman Long <long...@redhat.com> wrote: > v6: > - Add a second patch to rename the

Re: [PATCH v6 1/2] blktrace: Fix potentail deadlock between delete & sysfs ops

2017-09-19 Thread Steven Rostedt
On Tue, 19 Sep 2017 13:41:35 -0700 Christoph Hellwig wrote: > Call it blk_trace mutex and move it right next to the blk_trace > structure: > > ifdef CONFIG_BLK_DEV_IO_TRACE > struct blk_trace*blk_trace; > struct mutexblk_trace_mutex; >

Re: [PATCH v7] blktrace: Fix potentail deadlock between delete & sysfs ops

2017-09-20 Thread Steven Rostedt
Christoph, Can you give an acked-by for this patch? Jens, You want to take this through your tree, or do you want me to? If you want it, here's my: Acked-by: Steven Rostedt (VMware) <rost...@goodmis.org> -- Steve On Wed, 20 Sep 2017 13:26:11 -0400 Waiman Long <long...@redhat.c

Re: [PATCH v7] blktrace: Fix potentail deadlock between delete & sysfs ops

2017-09-20 Thread Steven Rostedt
On Wed, 20 Sep 2017 13:09:31 -0600 Jens Axboe wrote: > > I'll take it through my tree, and I'll prune some of that comment > as well (which should be a commit message thing, not a code comment). > Agreed, and thanks. -- Steve

Re: [PATCH v2 0/2] tracing/events: block: bring more on a par with blktrace

2018-04-13 Thread Steven Rostedt
; counterpart in blktrace tooling. > > The last patch is just an RFC. I still kept it in this patch set because > it is inspired by PATCH 2/2. > > Changes since v1: > [PATCH v2 1/2] > Use 0 and 1 instead of false and true for __print_symbolic table. > Now "trace-cmd report"

Re: [PATCH 1/2] tracing/events: block: track and print if unplug was explicit or schedule

2018-04-13 Thread Steven Rostedt
On Fri, 13 Apr 2018 15:07:17 +0200 Steffen Maier wrote: > Just like blktrace distinguishes explicit and schedule by means of > BLK_TA_UNPLUG_IO and BLK_TA_UNPLUG_TIMER, actually make use of the > existing argument "explicit" to distinguish the two cases in the one > common

Re: [RESEND PATCH v1 1/2] trace: events: scsi: Add tag in SCSI trace events

2018-04-16 Thread Steven Rostedt
On Mon, 16 Apr 2018 14:31:49 + "Bean Huo (beanhuo)" <bean...@micron.com> wrote: > Print the request tag along with other information > while tracing a command. > > Signed-off-by: Bean Huo <bean...@micron.com> > --- I don't see any issue with the tr

Re: [RESEND PATCH v1 2/2] trace: events: block: Add tag in block trace events

2018-04-16 Thread Steven Rostedt
ron.com> I don't see any issue with the tracing part. Acked-by: Steven Rostedt (VMware) <rost...@goodmis.org> Others need to check the content. -- Steve > --- > include/trace/events/block.h | 36 +--- > 1 file changed, 25 insertions(+), 11 deletions(-) >

Re: [RESEND PATCH v1 1/2] trace: events: scsi: Add tag in SCSI trace events

2018-04-16 Thread Steven Rostedt
On Mon, 16 Apr 2018 21:30:54 + Bart Van Assche wrote: > Hello Steve, > > The tool I'm most concerned about is blktrace. I'm not sure though how this > tool receives event data from the block layer core. Yeah, blktrace is "special", it looks like it registers its

Re: [RESEND PATCH v1 1/2] trace: events: scsi: Add tag in SCSI trace events

2018-04-16 Thread Steven Rostedt
On Mon, 16 Apr 2018 20:49:12 + Bart Van Assche wrote: > Which tools process these strings? Has it been verified whether or not > the tools that process these strings still work fine with this patch > applied? Ideally, tools shouldn't process trace event strings, but

Re: [RESEND PATCH v1 2/2] trace: events: block: Add tag in block trace events

2018-04-23 Thread Steven Rostedt
On Mon, 23 Apr 2018 14:43:13 +0200 Steffen Maier wrote: > > - TP_printk("[%s] %d", __entry->comm, __entry->nr_rq) > > + TP_printk("[%s] %d %s", __entry->comm, __entry->nr_rq, > > + __entry->explicit ? "Sync" : "Async") > > ); > > > > /** > > This

Re: [PATCH 1/3] blk-mq: use list_splice_tail() to insert requests

2018-06-28 Thread Steven Rostedt
On Thu, 28 Jun 2018 09:27:08 +0200 Johannes Thumshirn wrote: > Hi Ming, > > On Thu, Jun 28, 2018 at 11:19:16AM +0800, Ming Lei wrote: > > + list_for_each_entry(rq, list, queuelist) { > > BUG_ON(rq->mq_ctx != ctx); > > - list_del_init(>queuelist); > > -