linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kernel: export task_work_add
@ 2022-08-29  4:00 Ming Lei
  2022-09-07  0:44 ` Ming Lei
  2022-09-07 13:08 ` Bart Van Assche
  0 siblings, 2 replies; 6+ messages in thread
From: Ming Lei @ 2022-08-29  4:00 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-kernel, linux-block, Ming Lei, Christoph Hellwig

Firstly task_work_add() is used in several drivers. In ublk driver's
usage, request batching submission can only be applied with task_work_add,
and usually get better IOPS.

Secondly from this API's definition, the added work is always run in
the task context, and when task is exiting, either the work is rejected
to be added, or drained in do_exit(). In this way, not see obvious
disadvantage or potential issue by exporting it for module's usage.

So export it, then ublk driver can get simplified, meantime with better
performance.

Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 kernel/task_work.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/task_work.c b/kernel/task_work.c
index dff75bcde151..5f9a42a388f1 100644
--- a/kernel/task_work.c
+++ b/kernel/task_work.c
@@ -73,6 +73,7 @@ int task_work_add(struct task_struct *task, struct callback_head *work,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(task_work_add);
 
 /**
  * task_work_cancel_match - cancel a pending work added by task_work_add()
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] kernel: export task_work_add
  2022-08-29  4:00 [PATCH] kernel: export task_work_add Ming Lei
@ 2022-09-07  0:44 ` Ming Lei
  2022-09-07 13:08 ` Bart Van Assche
  1 sibling, 0 replies; 6+ messages in thread
From: Ming Lei @ 2022-09-07  0:44 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-kernel, linux-block, Christoph Hellwig

On Mon, Aug 29, 2022 at 12:00:13PM +0800, Ming Lei wrote:
> Firstly task_work_add() is used in several drivers. In ublk driver's
> usage, request batching submission can only be applied with task_work_add,
> and usually get better IOPS.
> 
> Secondly from this API's definition, the added work is always run in
> the task context, and when task is exiting, either the work is rejected
> to be added, or drained in do_exit(). In this way, not see obvious
> disadvantage or potential issue by exporting it for module's usage.
> 
> So export it, then ublk driver can get simplified, meantime with better
> performance.
> 
> Cc: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>

Hello Guys,

Gentle ping...


Thanks,
Ming


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kernel: export task_work_add
  2022-08-29  4:00 [PATCH] kernel: export task_work_add Ming Lei
  2022-09-07  0:44 ` Ming Lei
@ 2022-09-07 13:08 ` Bart Van Assche
  2022-09-07 13:44   ` Jens Axboe
  1 sibling, 1 reply; 6+ messages in thread
From: Bart Van Assche @ 2022-09-07 13:08 UTC (permalink / raw)
  To: Ming Lei, Jens Axboe; +Cc: linux-kernel, linux-block, Christoph Hellwig

On 8/28/22 21:00, Ming Lei wrote:
> Firstly task_work_add() is used in several drivers. In ublk driver's
> usage, request batching submission can only be applied with task_work_add,
> and usually get better IOPS.
> 
> Secondly from this API's definition, the added work is always run in
> the task context, and when task is exiting, either the work is rejected
> to be added, or drained in do_exit(). In this way, not see obvious
> disadvantage or potential issue by exporting it for module's usage.
> 
> So export it, then ublk driver can get simplified, meantime with better
> performance.

If task_work_add() is exported, shouldn't task_work_cancel() be exported
too? Anyway:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kernel: export task_work_add
  2022-09-07 13:08 ` Bart Van Assche
@ 2022-09-07 13:44   ` Jens Axboe
  2022-09-07 14:05     ` Christoph Hellwig
  0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2022-09-07 13:44 UTC (permalink / raw)
  To: Bart Van Assche, Ming Lei; +Cc: linux-kernel, linux-block, Christoph Hellwig

On 9/7/22 7:08 AM, Bart Van Assche wrote:
> On 8/28/22 21:00, Ming Lei wrote:
>> Firstly task_work_add() is used in several drivers. In ublk driver's
>> usage, request batching submission can only be applied with task_work_add,
>> and usually get better IOPS.
>>
>> Secondly from this API's definition, the added work is always run in
>> the task context, and when task is exiting, either the work is rejected
>> to be added, or drained in do_exit(). In this way, not see obvious
>> disadvantage or potential issue by exporting it for module's usage.
>>
>> So export it, then ublk driver can get simplified, meantime with better
>> performance.
> 
> If task_work_add() is exported, shouldn't task_work_cancel() be exported
> too? Anyway:

Not if it isn't currently used...

On the patch itself, it definitely makes sense in the context of ublk.
My hesitation is mostly around not really wanting to export this to
generic modular users. It's OK for core interfaces, of which ublk is
on the way to becoming, but I really don't like the idea of random
modules using it. But that's not really something we can manage with
the export, it's either exported or it's not...

-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kernel: export task_work_add
  2022-09-07 13:44   ` Jens Axboe
@ 2022-09-07 14:05     ` Christoph Hellwig
  2022-09-07 14:12       ` Jens Axboe
  0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2022-09-07 14:05 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Bart Van Assche, Ming Lei, linux-kernel, linux-block, Christoph Hellwig

On Wed, Sep 07, 2022 at 07:44:05AM -0600, Jens Axboe wrote:
> On the patch itself, it definitely makes sense in the context of ublk.
> My hesitation is mostly around not really wanting to export this to
> generic modular users. It's OK for core interfaces, of which ublk is
> on the way to becoming, but I really don't like the idea of random
> modules using it. But that's not really something we can manage with
> the export, it's either exported or it's not...

Yes, I'm really worried about folks doing stupid things with it.
Thinking of the whole loop saga..

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kernel: export task_work_add
  2022-09-07 14:05     ` Christoph Hellwig
@ 2022-09-07 14:12       ` Jens Axboe
  0 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2022-09-07 14:12 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Bart Van Assche, Ming Lei, linux-kernel, linux-block

On 9/7/22 8:05 AM, Christoph Hellwig wrote:
> On Wed, Sep 07, 2022 at 07:44:05AM -0600, Jens Axboe wrote:
>> On the patch itself, it definitely makes sense in the context of ublk.
>> My hesitation is mostly around not really wanting to export this to
>> generic modular users. It's OK for core interfaces, of which ublk is
>> on the way to becoming, but I really don't like the idea of random
>> modules using it. But that's not really something we can manage with
>> the export, it's either exported or it's not...
> 
> Yes, I'm really worried about folks doing stupid things with it.
> Thinking of the whole loop saga..

Exactly. But we don't really have any tools outside of clearly marking
it as such. It's not like we have an EXPORT_MODULE_CORE_GPL() and with
that requiring a driver or modular kernel feature that marks the module
as MODULE_IS_CORE_GPL().

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-09-07 14:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-29  4:00 [PATCH] kernel: export task_work_add Ming Lei
2022-09-07  0:44 ` Ming Lei
2022-09-07 13:08 ` Bart Van Assche
2022-09-07 13:44   ` Jens Axboe
2022-09-07 14:05     ` Christoph Hellwig
2022-09-07 14:12       ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).