[PATCH v17 01/47] llist: move llist_{head,node} definition to types.h

Byungchul Park posted 47 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH v17 01/47] llist: move llist_{head,node} definition to types.h
Posted by Byungchul Park 2 months, 2 weeks ago
llist_head and llist_node can be used by some other header files.  For
example, dept for tracking dependencies uses llist in its header.  To
avoid header dependency, move them to types.h.

Signed-off-by: Byungchul Park <byungchul@sk.com>
---
 include/linux/llist.h | 8 --------
 include/linux/types.h | 8 ++++++++
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/linux/llist.h b/include/linux/llist.h
index 607b2360c938..6bcdf378ebd7 100644
--- a/include/linux/llist.h
+++ b/include/linux/llist.h
@@ -53,14 +53,6 @@
 #include <linux/stddef.h>
 #include <linux/types.h>
 
-struct llist_head {
-	struct llist_node *first;
-};
-
-struct llist_node {
-	struct llist_node *next;
-};
-
 #define LLIST_HEAD_INIT(name)	{ NULL }
 #define LLIST_HEAD(name)	struct llist_head name = LLIST_HEAD_INIT(name)
 
diff --git a/include/linux/types.h b/include/linux/types.h
index 6dfdb8e8e4c3..58882a3730eb 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -208,6 +208,14 @@ struct hlist_node {
 	struct hlist_node *next, **pprev;
 };
 
+struct llist_head {
+	struct llist_node *first;
+};
+
+struct llist_node {
+	struct llist_node *next;
+};
+
 struct ustat {
 	__kernel_daddr_t	f_tfree;
 #ifdef CONFIG_ARCH_32BIT_USTAT_F_TINODE
-- 
2.17.1
Re: [PATCH v17 01/47] llist: move llist_{head,node} definition to types.h
Posted by Greg KH 2 months, 2 weeks ago
On Thu, Oct 02, 2025 at 05:12:01PM +0900, Byungchul Park wrote:
> llist_head and llist_node can be used by some other header files.  For
> example, dept for tracking dependencies uses llist in its header.  To
> avoid header dependency, move them to types.h.

If you need llist in your code, then include llist.h.  Don't force all
types.h users to do so as there is not a dependency in types.h for
llist.h.

This patch shouldn't be needed as you are hiding "header dependency" for
other files.

thanks,

greg k-h
Re: [PATCH v17 01/47] llist: move llist_{head,node} definition to types.h
Posted by Byungchul Park 2 months ago
On Thu, Oct 02, 2025 at 10:24:41AM +0200, Greg KH wrote:
> On Thu, Oct 02, 2025 at 05:12:01PM +0900, Byungchul Park wrote:
> > llist_head and llist_node can be used by some other header files.  For
> > example, dept for tracking dependencies uses llist in its header.  To
> > avoid header dependency, move them to types.h.
> 
> If you need llist in your code, then include llist.h.  Don't force all

Eventually, I found out another way to avoid the dependency issue.
Thanks anyway for the feedback.

	Byungchul

> types.h users to do so as there is not a dependency in types.h for
> llist.h.
> 
> This patch shouldn't be needed as you are hiding "header dependency" for
> other files.
> 
> thanks,
> 
> greg k-h
Re: [PATCH v17 01/47] llist: move llist_{head,node} definition to types.h
Posted by Mathieu Desnoyers 2 months, 2 weeks ago
On 2025-10-02 04:24, Greg KH wrote:
> On Thu, Oct 02, 2025 at 05:12:01PM +0900, Byungchul Park wrote:
>> llist_head and llist_node can be used by some other header files.  For
>> example, dept for tracking dependencies uses llist in its header.  To
>> avoid header dependency, move them to types.h.
> 
> If you need llist in your code, then include llist.h.  Don't force all
> types.h users to do so as there is not a dependency in types.h for
> llist.h.
> 
> This patch shouldn't be needed as you are hiding "header dependency" for
> other files.

I agree that moving this into a catch-all types.h is not what we should
aim for.

However, it's a good practice to move the type declarations to a
separate header file, so code that only cares about type and not
implementation of static inline functions can include just that.

Perhaps we can move struct llist_head and struct llist_node to a new
include/linux/llist_types.h instead ?

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
Re: [PATCH v17 01/47] llist: move llist_{head,node} definition to types.h
Posted by Arnd Bergmann 2 months, 2 weeks ago
On Thu, Oct 2, 2025, at 15:53, Mathieu Desnoyers wrote:
> On 2025-10-02 04:24, Greg KH wrote:
>> On Thu, Oct 02, 2025 at 05:12:01PM +0900, Byungchul Park wrote:
>>> llist_head and llist_node can be used by some other header files.  For
>>> example, dept for tracking dependencies uses llist in its header.  To
>>> avoid header dependency, move them to types.h.
>> 
>> If you need llist in your code, then include llist.h.  Don't force all
>> types.h users to do so as there is not a dependency in types.h for
>> llist.h.
>> 
>> This patch shouldn't be needed as you are hiding "header dependency" for
>> other files.
>
> I agree that moving this into a catch-all types.h is not what we should
> aim for.
>
> However, it's a good practice to move the type declarations to a
> separate header file, so code that only cares about type and not
> implementation of static inline functions can include just that.
>
> Perhaps we can move struct llist_head and struct llist_node to a new
> include/linux/llist_types.h instead ?

We have around a dozen types of linked lists, and the most common
two of them are currently defined in linux/types.h, while the
rest of them are each defined in the same header as the inteface
definition.

Duplicating each of those headers by splitting out the trivial
type definition doesn't quite seem right either, as we'd end
up with even more headers that have to be included indirectly
in each compilation unit.

Maybe a shared linux/list_types.h would work, to specifically
contain all the list_head variants that are meant to be included
in larger structures?

    Arnd
Re: [PATCH v17 01/47] llist: move llist_{head,node} definition to types.h
Posted by Byungchul Park 2 months ago
On Fri, Oct 03, 2025 at 01:19:33AM +0200, Arnd Bergmann wrote:
> On Thu, Oct 2, 2025, at 15:53, Mathieu Desnoyers wrote:
> > On 2025-10-02 04:24, Greg KH wrote:
> >> On Thu, Oct 02, 2025 at 05:12:01PM +0900, Byungchul Park wrote:
> >>> llist_head and llist_node can be used by some other header files.  For
> >>> example, dept for tracking dependencies uses llist in its header.  To
> >>> avoid header dependency, move them to types.h.
> >>
> >> If you need llist in your code, then include llist.h.  Don't force all
> >> types.h users to do so as there is not a dependency in types.h for
> >> llist.h.
> >>
> >> This patch shouldn't be needed as you are hiding "header dependency" for
> >> other files.
> >
> > I agree that moving this into a catch-all types.h is not what we should
> > aim for.
> >
> > However, it's a good practice to move the type declarations to a
> > separate header file, so code that only cares about type and not
> > implementation of static inline functions can include just that.
> >
> > Perhaps we can move struct llist_head and struct llist_node to a new
> > include/linux/llist_types.h instead ?
> 
> We have around a dozen types of linked lists, and the most common
> two of them are currently defined in linux/types.h, while the
> rest of them are each defined in the same header as the inteface
> definition.
> 
> Duplicating each of those headers by splitting out the trivial
> type definition doesn't quite seem right either, as we'd end
> up with even more headers that have to be included indirectly
> in each compilation unit.
> 
> Maybe a shared linux/list_types.h would work, to specifically

I found a way to resolve my issue, but I thought it's good idea
regardless of my issue and took a quick look.  However, it seems like
there's an overwhelming amount of work since it might require to replace
all the existing include <linux/types.h> for use of list things with the
new one :-).

	Byungchul

> contain all the list_head variants that are meant to be included
> in larger structures?
> 
>     Arnd
Re: [PATCH v17 01/47] llist: move llist_{head,node} definition to types.h
Posted by Arnd Bergmann 2 months ago
On Thu, Oct 16, 2025, at 02:46, Byungchul Park wrote:
> On Fri, Oct 03, 2025 at 01:19:33AM +0200, Arnd Bergmann wrote:
>> On Thu, Oct 2, 2025, at 15:53, Mathieu Desnoyers wrote:
>> > On 2025-10-02 04:24, Greg KH wrote:
>> >> On Thu, Oct 02, 2025 at 05:12:01PM +0900, Byungchul Park wrote:

>> Maybe a shared linux/list_types.h would work, to specifically
>
> I found a way to resolve my issue, but I thought it's good idea
> regardless of my issue and took a quick look.  However, it seems like
> there's an overwhelming amount of work since it might require to replace
> all the existing include <linux/types.h> for use of list things with the
> new one :-).

I don't think it's that bad, since almost every header ends up
including linux/list.h indirectly at the moment.

A little bit of scripting to find the headers that reference
'struct list_head' but don't also include linux/list.h reveals
this relatively short set that would need to include the new
header:

> include/keys/asymmetric-parser.h
> include/linux/dynamic_debug.h
> include/linux/genalloc.h
> include/linux/gpio/machine.h
> include/linux/hiddev.h
> include/linux/iio/iio-opaque.h
> include/linux/iio/sysfs.h
> include/linux/input/touch-overlay.h
> include/linux/irq_poll.h
> include/linux/iscsi_boot_sysfs.h
> include/linux/kcore.h
> include/linux/kcsan-checks.h
> include/linux/kcsan.h
> include/linux/lockdep_types.h
> include/linux/logic_pio.h
> include/linux/maple.h
> include/linux/mfd/iqs62x.h
> include/linux/mlx5/macsec.h
> include/linux/mount.h
> include/linux/mtd/map.h
> include/linux/mtd/nand-qpic-common.h
> include/linux/mtd/partitions.h
> include/linux/mutex_types.h
> include/linux/nfs_fs_i.h
> include/linux/of_iommu.h
> include/linux/parport_pc.h
> include/linux/pinctrl/pinctrl.h
> include/linux/plist_types.h
> include/linux/pm_wakeup.h
> include/linux/reboot-mode.h
> include/linux/shm.h
> include/linux/smpboot.h
> include/linux/sunrpc/xprtmultipath.h
> include/linux/usb/audio.h
> include/linux/workqueue_types.h
> include/linux/zpool.h
> include/net/bluetooth/hci_sync.h
> include/net/bluetooth/l2cap.h
> include/net/bluetooth/rfcomm.h
> include/net/dcbnl.h
> include/sound/i2c.h
> include/sound/soc-jack.h
> include/target/iscsi/iscsi_transport.h
> include/video/udlfb.h

A lot of these don't have any #include statements at all,
which indicates that they expect to only be included in
places where the dependencies are already visible.

      Arnd