Re: [PATCH 04/16] block: use atomic bitops for ->queue_flags

2018-11-15 Thread Christoph Hellwig
On Thu, Nov 15, 2018 at 07:55:02AM +0100, Hannes Reinecke wrote:
>> Signed-off-by: Christoph Hellwig 
>> ---
>>   block/blk-core.c   | 54 ++--
>>   block/blk-mq.c |  2 +-
>>   block/blk-settings.c   | 10 +++-
>>   block/blk-sysfs.c  | 28 +
>>   block/blk.h| 56 --
>>   include/linux/blkdev.h |  1 -
>>   6 files changed, 24 insertions(+), 127 deletions(-)
>>
> I wonder if we can't remove the 'blk_queue_flag_XXX' helpers and replace 
> them with inlines ...

I'd prefer to just open code the *bit helpers as that is a lot more
reasonable.  But that is quite a lot of churn, so I'll only do that if
I get previous buy-in to that idea, and preferably in a separate series.


Re: [PATCH 04/16] block: use atomic bitops for ->queue_flags

2018-11-14 Thread Hannes Reinecke

On 11/14/18 5:02 PM, Christoph Hellwig wrote:

->queue_flags is generally not set or cleared in the fast path, and also
generally set or cleared one flag at a time.  Make use of the normal
atomic bitops for it so that we don't need to take the queue_lock,
which is otherwise mostly unused in the core block layer now.

Signed-off-by: Christoph Hellwig 
---
  block/blk-core.c   | 54 ++--
  block/blk-mq.c |  2 +-
  block/blk-settings.c   | 10 +++-
  block/blk-sysfs.c  | 28 +
  block/blk.h| 56 --
  include/linux/blkdev.h |  1 -
  6 files changed, 24 insertions(+), 127 deletions(-)

I wonder if we can't remove the 'blk_queue_flag_XXX' helpers and replace 
them with inlines ...


Otherwise:

Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)


[PATCH 04/16] block: use atomic bitops for ->queue_flags

2018-11-14 Thread Christoph Hellwig
->queue_flags is generally not set or cleared in the fast path, and also
generally set or cleared one flag at a time.  Make use of the normal
atomic bitops for it so that we don't need to take the queue_lock,
which is otherwise mostly unused in the core block layer now.

Signed-off-by: Christoph Hellwig 
---
 block/blk-core.c   | 54 ++--
 block/blk-mq.c |  2 +-
 block/blk-settings.c   | 10 +++-
 block/blk-sysfs.c  | 28 +
 block/blk.h| 56 --
 include/linux/blkdev.h |  1 -
 6 files changed, 24 insertions(+), 127 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 1c9b6975cf0a..5c8e66a09d82 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -74,11 +74,7 @@ static struct workqueue_struct *kblockd_workqueue;
  */
 void blk_queue_flag_set(unsigned int flag, struct request_queue *q)
 {
-   unsigned long flags;
-
-   spin_lock_irqsave(q->queue_lock, flags);
-   queue_flag_set(flag, q);
-   spin_unlock_irqrestore(q->queue_lock, flags);
+   set_bit(flag, &q->queue_flags);
 }
 EXPORT_SYMBOL(blk_queue_flag_set);
 
@@ -89,11 +85,7 @@ EXPORT_SYMBOL(blk_queue_flag_set);
  */
 void blk_queue_flag_clear(unsigned int flag, struct request_queue *q)
 {
-   unsigned long flags;
-
-   spin_lock_irqsave(q->queue_lock, flags);
-   queue_flag_clear(flag, q);
-   spin_unlock_irqrestore(q->queue_lock, flags);
+   clear_bit(flag, &q->queue_flags);
 }
 EXPORT_SYMBOL(blk_queue_flag_clear);
 
@@ -107,38 +99,10 @@ EXPORT_SYMBOL(blk_queue_flag_clear);
  */
 bool blk_queue_flag_test_and_set(unsigned int flag, struct request_queue *q)
 {
-   unsigned long flags;
-   bool res;
-
-   spin_lock_irqsave(q->queue_lock, flags);
-   res = queue_flag_test_and_set(flag, q);
-   spin_unlock_irqrestore(q->queue_lock, flags);
-
-   return res;
+   return test_and_set_bit(flag, &q->queue_flags);
 }
 EXPORT_SYMBOL_GPL(blk_queue_flag_test_and_set);
 
-/**
- * blk_queue_flag_test_and_clear - atomically test and clear a queue flag
- * @flag: flag to be cleared
- * @q: request queue
- *
- * Returns the previous value of @flag - 0 if the flag was not set and 1 if
- * the flag was set.
- */
-bool blk_queue_flag_test_and_clear(unsigned int flag, struct request_queue *q)
-{
-   unsigned long flags;
-   bool res;
-
-   spin_lock_irqsave(q->queue_lock, flags);
-   res = queue_flag_test_and_clear(flag, q);
-   spin_unlock_irqrestore(q->queue_lock, flags);
-
-   return res;
-}
-EXPORT_SYMBOL_GPL(blk_queue_flag_test_and_clear);
-
 void blk_rq_init(struct request_queue *q, struct request *rq)
 {
memset(rq, 0, sizeof(*rq));
@@ -368,12 +332,10 @@ void blk_cleanup_queue(struct request_queue *q)
/* mark @q DYING, no new request or merges will be allowed afterwards */
mutex_lock(&q->sysfs_lock);
blk_set_queue_dying(q);
-   spin_lock_irq(lock);
 
-   queue_flag_set(QUEUE_FLAG_NOMERGES, q);
-   queue_flag_set(QUEUE_FLAG_NOXMERGES, q);
-   queue_flag_set(QUEUE_FLAG_DYING, q);
-   spin_unlock_irq(lock);
+   blk_queue_flag_set(QUEUE_FLAG_NOMERGES, q);
+   blk_queue_flag_set(QUEUE_FLAG_NOXMERGES, q);
+   blk_queue_flag_set(QUEUE_FLAG_DYING, q);
mutex_unlock(&q->sysfs_lock);
 
/*
@@ -384,9 +346,7 @@ void blk_cleanup_queue(struct request_queue *q)
 
rq_qos_exit(q);
 
-   spin_lock_irq(lock);
-   queue_flag_set(QUEUE_FLAG_DEAD, q);
-   spin_unlock_irq(lock);
+   blk_queue_flag_set(QUEUE_FLAG_DEAD, q);
 
/*
 * make sure all in-progress dispatch are completed because
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 4c82b4b4fa3e..e2717e843727 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2756,7 +2756,7 @@ struct request_queue *blk_mq_init_allocated_queue(struct 
blk_mq_tag_set *set,
q->queue_flags |= QUEUE_FLAG_MQ_DEFAULT;
 
if (!(set->flags & BLK_MQ_F_SG_MERGE))
-   queue_flag_set_unlocked(QUEUE_FLAG_NO_SG_MERGE, q);
+   blk_queue_flag_set(QUEUE_FLAG_NO_SG_MERGE, q);
 
q->sg_reserved_size = INT_MAX;
 
diff --git a/block/blk-settings.c b/block/blk-settings.c
index cca83590a1dc..3abe831e92c8 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -834,16 +834,14 @@ EXPORT_SYMBOL(blk_set_queue_depth);
  */
 void blk_queue_write_cache(struct request_queue *q, bool wc, bool fua)
 {
-   spin_lock_irq(q->queue_lock);
if (wc)
-   queue_flag_set(QUEUE_FLAG_WC, q);
+   blk_queue_flag_set(QUEUE_FLAG_WC, q);
else
-   queue_flag_clear(QUEUE_FLAG_WC, q);
+   blk_queue_flag_clear(QUEUE_FLAG_WC, q);
if (fua)
-   queue_flag_set(QUEUE_FLAG_FUA, q);
+   blk_queue_flag_set(QUEUE_FLAG_FUA, q);
else
-   queue_flag_clear(QUEUE_FLAG_FUA, q);
-   spin_unlock_irq(q->qu