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

2017-08-10 Thread Waiman Long
delete operation is in progress. Signed-off-by: Waiman Long <long...@redhat.com> --- block/ioctl.c | 3 +++ include/linux/fs.h | 1 + kernel/trace/blktrace.c | 24 ++-- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/block/ioctl.c b/block/i

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

2017-08-18 Thread Waiman Long
won't go away as device deletion requires all the sysfs references to be gone. Therefore, there is no need to take the bd_mutex. Instead, a global blktrace mutex will be used to serialize the read/write of the blktrace sysfs attributes. Signed-off-by: Waiman Long <long...@redhat.com> -

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

2017-08-18 Thread Waiman Long
On 08/18/2017 02:07 PM, Bart Van Assche wrote: > On Fri, 2017-08-18 at 13:54 -0400, Waiman Long wrote: >> Instead, a global blktrace >> mutex will be used to serialize the read/write of the blktrace sysfs >> attributes. > Hello Waiman, > > Using a mutex to serialize

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

2017-08-18 Thread Waiman Long
On 08/18/2017 04:18 PM, Bart Van Assche wrote: > On Fri, 2017-08-18 at 16:01 -0400, Waiman Long wrote: >> [ ... ] >> Instead of using bd_mutex, a new global blktrace mutex is now used >> to protect against concurrent access, creation and destruction of the >> blk_trace

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

2017-08-18 Thread Waiman Long
ce.c file. As blktrace files will not be frequently accessed, using a global mutex should not cause any performance problem. Signed-off-by: Waiman Long <long...@redhat.com> --- v4: - Use blktrace_mutex in blk_trace_ioctl() as well. v3: - Use a global blktrace_mutex to serialize sysfs att

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

2017-08-18 Thread Waiman Long
On 08/18/2017 12:21 PM, Bart Van Assche wrote: > On Fri, 2017-08-18 at 09:55 -0400, Waiman Long wrote: >> On 08/17/2017 05:30 PM, Steven Rostedt wrote: >>> On Thu, 17 Aug 2017 17:10:07 -0400 >>> Steven Rostedt <rost...@goodmis.org> wrote: >>>>

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

2017-08-17 Thread Waiman Long
On 08/17/2017 05:10 PM, Steven Rostedt wrote: > On Thu, 17 Aug 2017 12:24:39 -0400 > Waiman Long <long...@redhat.com> wrote: > >>>> + * sysfs file and then acquiring the bd_mutex. Deleting a block device >>>> + * requires acquiring the bd_m

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

2017-08-18 Thread Waiman Long
On 08/17/2017 05:30 PM, Steven Rostedt wrote: > On Thu, 17 Aug 2017 17:10:07 -0400 > Steven Rostedt 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

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

2017-09-16 Thread Waiman Long
lktrace.c file. There is no point in adding one more mutex to the block_device structure just for blktrace. Signed-off-by: Waiman Long <long...@redhat.com> --- v5: - Overload the bd_fsfreeze_mutex in block_device structure for blktrace protection. v4: - Use blktrace_mutex in

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

2017-09-18 Thread Waiman Long
lktrace.c file. There is no point in adding one more mutex to the block_device structure just for blktrace. Signed-off-by: Waiman Long <long...@redhat.com> --- include/linux/fs.h | 2 +- kernel/trace/blktrace.c | 26 -- 2 files changed, 21 insertions(+), 7 del

[PATCH v6 2/2] block_dev: Rename bd_fsfreeze_mutex

2017-09-18 Thread Waiman Long
As the bd_fsfreeze_mutex is used by the blktrace subsystem as well, it is now renamed to bd_fsfreeze_blktrace_mutex to better reflect its purpose. Signed-off-by: Waiman Long <long...@redhat.com> --- fs/block_dev.c | 14 +++--- fs/gfs2/ops_fstype.c| 6 +++--- fs/

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

2017-09-18 Thread Waiman Long
between block device deletion and sysfs operations. Waiman Long (2): blktrace: Fix potentail deadlock between delete & sysfs ops block_dev: Rename bd_fsfreeze_mutex fs/block_dev.c | 14 +++--- fs/gfs2/ops_fstype.c| 6 +++--- fs/nilfs2/super.c | 6 +++--- fs/sup

Re: [PATCH v6 2/2] block_dev: Rename bd_fsfreeze_mutex

2017-09-19 Thread Waiman Long
On 09/18/2017 07:47 PM, Christoph Hellwig wrote: > Don't rename it to a way to long name. Either add a separate mutex > for your purpose (unless there is interaction between freezing and > blktrace, which I doubt), or properly comment the usage. I would agree with you if the long name causes the

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

2017-09-19 Thread Waiman Long
On 09/18/2017 08:01 PM, Christoph Hellwig wrote: > Taking a look at this it seems like using a lock in struct block_device > isn't the right thing to do anyway - all the action is on fields in > struct blk_trace, so having a lock inside that would make a lot more > sense. > > It would also help to

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

2017-09-20 Thread Waiman Long
On 09/20/2017 01:35 PM, Christoph Hellwig wrote: >> +/* >> + * When reading or writing the blktrace sysfs files, the references to the >> + * opened sysfs or device files should prevent the underlying block device >> + * from being removed. So no further delete protection is really needed. >> + *

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

2017-09-20 Thread Waiman Long
ellwig <h...@infradead.org> Signed-off-by: Waiman Long <long...@redhat.com> --- v7: - Add a new blk_trace_mutex in request_queue structure for blk_trace protection. v6: - Add a second patch to rename the bd_fsfreeze_mutex to bd_fsfreeze_blktrace_mutex. v5: - Overload

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

2017-09-19 Thread Waiman Long
On 09/19/2017 10:38 AM, Christoph Hellwig wrote: > On Tue, Sep 19, 2017 at 08:49:12AM -0400, Waiman Long wrote: >> On 09/18/2017 08:01 PM, Christoph Hellwig wrote: >>> Taking a look at this it seems like using a lock in struct block_device >>> isn't the right thing to