Re: [PATCH v2] swait: export symbols __prepare_to_swait and __finish_swait

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Wed, 23 May 2018, Mike Snitzer wrote:

> [Peter, in this v2 I switched to using _GPL for the exports and updated
> the patch header.  As covered in previous mail, please let me know if
> you're OK with me staging this change for 4.18 via linux-dm.git with
> your Ack, thanks]
> 
> From: Mikulas Patocka <mpatocka@xxxxxxxxxx>
> Subject: [PATCH] swait: export symbols __prepare_to_swait and __finish_swait
> 
> __prepare_to_swait and __finish_swait are declared in
> include/linux/swait.h but they are not exported; so they are not useable
> from kernel modules.
> 
> A new consumer of swait (in dm-writecache) reduces its locking overhead
> by using the spinlock in swait_queue_head to protect not only the wait
> queue, but also the list of events.  Consequently, this swait consuming
> kernel module needs to use these unlocked functions.
> 
> Peter Zijlstra explained:
>   "The reason swait exists is to be deterministic (for RT) -- something
>   that regular wait code cannot be.
>   And by (ab)using / exporting the wait internal lock you risk losing
>   that. So I don't think the proposed [dm-writecache] usage is bad, it
>   is possible to create badness.
>   So if we're going to export them; someone needs to keep an eye on things
>   and ensure the lock isn't abused."
> 
> So while this new use of the wait internal lock doesn't jeopardize the
> realtime requirements of swait, these exports do open swait's internal
> locking up to being abused.  As such, EXPORT_SYMBOL_GPL is used because
> any future consumers of __prepare_to_swait and __finish_swait must
> always be thoroughly scrutinized.
> 
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>
> Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx>
> ---
>  kernel/sched/swait.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/sched/swait.c b/kernel/sched/swait.c
> index b6fb2c3b3ff7..5d891b65ada5 100644
> --- a/kernel/sched/swait.c
> +++ b/kernel/sched/swait.c
> @@ -75,6 +75,7 @@ void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait)
>  	if (list_empty(&wait->task_list))
>  		list_add(&wait->task_list, &q->task_list);
>  }
> +EXPORT_SYMBOL_GPL(__prepare_to_swait);
>  
>  void prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait, int state)
>  {
> @@ -104,6 +105,7 @@ void __finish_swait(struct swait_queue_head *q, struct swait_queue *wait)
>  	if (!list_empty(&wait->task_list))
>  		list_del_init(&wait->task_list);
>  }
> +EXPORT_SYMBOL_GPL(__finish_swait);
>  
>  void finish_swait(struct swait_queue_head *q, struct swait_queue *wait)
>  {
> -- 
> 2.15.0

Then - you should export swake_up_locked with EXPORT_SYMBOL_GPL too.

Because swake_up_locked is unusable without __prepare_to_swait and 
__finish_swait.

Mikulas

--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel



[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux