Re: [RESEND PATCH] kernfs: Avoid re-adding kernfs_node into kernfs_notify_list.

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

 



Hello Tejun,

On 6/7/22 4:33 am, Tejun Heo wrote:
> Hello,
> 
> On Sun, Jul 03, 2022 at 09:09:05PM +1000, Imran Khan wrote:
>> Can we use kernfs_notify_lock like below snippet to serialize producers
>> (kernfs_notify):
>>
>> spin_lock_irqsave(&kernfs_notify_lock, flags);
>> if (kn->attr.notify_next.next != NULL) {
>> 	kernfs_get(kn);
>> 	llist_add(&kn->attr.notify_next, &kernfs_notify_list);
>> 	schedule_work(&kernfs_notify_work);
>> }
>> spin_unlock_irqsave(&kernfs_notify_lock, flags);
> 
> But then what's the point of using llist?
> 

In this case, the point of using llist would be to avoid taking the locks in
consumer.
>> As per following comments at the beginning of llist.h
>>
>>  * Cases where locking is not needed:
>>  * If there are multiple producers and multiple consumers, llist_add can be
>>  * used in producers and llist_del_all can be used in consumers simultaneously
>>  * without locking. Also a single consumer can use llist_del_first while
>>  * multiple producers simultaneously use llist_add, without any locking.
>>
>> Multiple producers and single consumer can work in parallel but as in our case
>> addition is dependent on kn->attr.notify_next.next != NULL, we may keep the
>> checking and list addition under kernfs_notify_lock and for consumer just lock
>> free->next = NULL under kernfs_notify_lock.
> 
> It supports multiple producers in the sense that multiple producers can try
> to add their own llist_nodes concurrently. It doesn't support multiple
> producers trying to add the same llist_node whether that depends on NULL
> check or not.
> 

Hmm. My idea was that eventually we will never run into situation where multiple
producers will end up adding the same node because as soon as first producer
adds the node (the other potential adders are spinning on kernfs_notify_lock),
kn->attr.notif_next.next will get a non-NULL value and checking
(kn->attr.notify_next.next != NULL) will avoid the node getting re-added.

I must be missing something here, so as per your suggestion I have reverted this
change at [1].

Thanks,
 -- Imran

[1]: https://lore.kernel.org/lkml/20220705201026.2487665-1-imran.f.khan@xxxxxxxxxx/



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux