Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop
On Wed, Jul 29, 2020 at 06:04:57PM +0300, Andy Shevchenko wrote: > On Wed, Jul 29, 2020 at 4:35 PM Waiman Long wrote: > > On 7/29/20 8:28 AM, Herbert Xu wrote: > > ... > > > This patch series looks good to me. I just wonder if we should also move > > ATOMIC64_INIT() to types.h for symmetry purpose. Anyway, > > Same question here. Yes I almost started doing it but at least one architecture (arc) had a custom atomic64_t so I kept it out just to be on the safe side. We certainly could do this as a follow-up patch. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop
On Thu, Jul 30, 2020 at 4:00 AM Herbert Xu wrote: > > On Wed, Jul 29, 2020 at 06:04:57PM +0300, Andy Shevchenko wrote: > > On Wed, Jul 29, 2020 at 4:35 PM Waiman Long wrote: > > > On 7/29/20 8:28 AM, Herbert Xu wrote: > > > > ... > > > > > This patch series looks good to me. I just wonder if we should also move > > > ATOMIC64_INIT() to types.h for symmetry purpose. Anyway, > > > > Same question here. > > Yes I almost started doing it but at least one architecture (arc) > had a custom atomic64_t so I kept it out just to be on the safe > side. We may ask Synopsys folks to look at this as well. Vineet, any ideas if we may unify ATOMIC64_INIT() across the architectures? > We certainly could do this as a follow-up patch. -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop
On Thu, Jul 30, 2020 at 10:47:16AM +0300, Andy Shevchenko wrote: > > We may ask Synopsys folks to look at this as well. > Vineet, any ideas if we may unify ATOMIC64_INIT() across the architectures? I don't think there is any technical difficulty. The custom atomic64_t simply adds an alignment requirement so the initialisor remains the same. I just didn't want to add more complexity to the existing patch. As a follow-up patch it should be quite straightforward. Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop
On Thu, Jul 30, 2020 at 10:51 AM Herbert Xu wrote: > On Thu, Jul 30, 2020 at 10:47:16AM +0300, Andy Shevchenko wrote: > > > > We may ask Synopsys folks to look at this as well. > > Vineet, any ideas if we may unify ATOMIC64_INIT() across the architectures? > > I don't think there is any technical difficulty. The custom > atomic64_t simply adds an alignment requirement so the initialisor > remains the same. > > I just didn't want to add more complexity to the existing patch. > As a follow-up patch it should be quite straightforward. Ah, I see what you mean. I considered the follow up patch as well, I thought there were some bigger impediments. -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop
On 7/30/20 12:50 AM, Herbert Xu wrote: > On Thu, Jul 30, 2020 at 10:47:16AM +0300, Andy Shevchenko wrote: >> We may ask Synopsys folks to look at this as well. >> Vineet, any ideas if we may unify ATOMIC64_INIT() across the architectures? > I don't think there is any technical difficulty. The custom > atomic64_t simply adds an alignment requirement so the initialisor > remains the same. Exactly so. FWIW the alignment requirement is because ARC ABI allows 64-bit data to be 32-bit aligned provided hardware deals fine with 4 byte aligned for the non-atomic double-load/store LDD/STD instructions. The 64-bit alignement however is required for atomic double load/store LLOCKD/SCONDD instructions hence the definition of ARC atomic64_t -Vineet
Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop
On Wed, Jul 29, 2020 at 10:28:07PM +1000, Herbert Xu wrote: > This miniseries breaks a header loop involving qspinlock_types.h. > The issue is that qspinlock_types.h includes atomic.h, which then > eventually includes kernel.h which could lead back to the original > file via spinlock_types.h. How did you run into this, I haven't seen any build failures due to this. > The first patch moves ATOMIC_INIT into linux/types.h while the second > patch actuallys breaks the loop by no longer including atomic.h > in qspinlock_types.h. Anyway, the patches look sane enough, I'll go stick them in tip/locking/core or somesuch.
Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop
On Wed, Jul 29, 2020 at 02:47:44PM +0200, pet...@infradead.org wrote: > > How did you run into this, I haven't seen any build failures due to > this. I didn't, Stephen ran into it after merging the printk tree together with the powerpc tree: https://lore.kernel.org/lkml/20200729210311.425d0...@canb.auug.org.au/ > Anyway, the patches look sane enough, I'll go stick them in > tip/locking/core or somesuch. Perhaps add them on top of the other two patches in locking/header? Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop
On Wed, Jul 29, 2020 at 10:51:44PM +1000, Herbert Xu wrote: > On Wed, Jul 29, 2020 at 02:47:44PM +0200, pet...@infradead.org wrote: > > > > How did you run into this, I haven't seen any build failures due to > > this. > > I didn't, Stephen ran into it after merging the printk tree together > with the powerpc tree: > > https://lore.kernel.org/lkml/20200729210311.425d0...@canb.auug.org.au/ > > > Anyway, the patches look sane enough, I'll go stick them in > > tip/locking/core or somesuch. > > Perhaps add them on top of the other two patches in locking/header? Can do,
Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop
On (20/07/29 15:00), pet...@infradead.org wrote: > On Wed, Jul 29, 2020 at 10:51:44PM +1000, Herbert Xu wrote: > > On Wed, Jul 29, 2020 at 02:47:44PM +0200, pet...@infradead.org wrote: > > > [..] > > > Anyway, the patches look sane enough, I'll go stick them in > > > tip/locking/core or somesuch. > > > > Perhaps add them on top of the other two patches in locking/header? > > Can do, locking/header would be better -ss
Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop
On 7/29/20 8:28 AM, Herbert Xu wrote: This miniseries breaks a header loop involving qspinlock_types.h. The issue is that qspinlock_types.h includes atomic.h, which then eventually includes kernel.h which could lead back to the original file via spinlock_types.h. The first patch moves ATOMIC_INIT into linux/types.h while the second patch actuallys breaks the loop by no longer including atomic.h in qspinlock_types.h. Cheers, This patch series looks good to me. I just wonder if we should also move ATOMIC64_INIT() to types.h for symmetry purpose. Anyway, Acked-by: Waiman Long
Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop
On Wed, Jul 29, 2020 at 10:28:49PM +0900, Sergey Senozhatsky wrote: > On (20/07/29 15:00), pet...@infradead.org wrote: > > On Wed, Jul 29, 2020 at 10:51:44PM +1000, Herbert Xu wrote: > > > On Wed, Jul 29, 2020 at 02:47:44PM +0200, pet...@infradead.org wrote: > > > > > [..] > > > > Anyway, the patches look sane enough, I'll go stick them in > > > > tip/locking/core or somesuch. > > > > > > Perhaps add them on top of the other two patches in locking/header? > > > > Can do, > > locking/header would be better Done.
Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop
On Wed, Jul 29, 2020 at 4:35 PM Waiman Long wrote: > On 7/29/20 8:28 AM, Herbert Xu wrote: ... > This patch series looks good to me. I just wonder if we should also move > ATOMIC64_INIT() to types.h for symmetry purpose. Anyway, Same question here. -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop
On (20/07/29 16:28), pet...@infradead.org wrote: > On Wed, Jul 29, 2020 at 10:28:49PM +0900, Sergey Senozhatsky wrote: > > On (20/07/29 15:00), pet...@infradead.org wrote: > > > On Wed, Jul 29, 2020 at 10:51:44PM +1000, Herbert Xu wrote: > > > > On Wed, Jul 29, 2020 at 02:47:44PM +0200, pet...@infradead.org wrote: > > > > > > > [..] > > > > > Anyway, the patches look sane enough, I'll go stick them in > > > > > tip/locking/core or somesuch. > > > > > > > > Perhaps add them on top of the other two patches in locking/header? > > > > > > Can do, > > > > locking/header would be better > > Done. Thanks a lot! I'll run some cross-compilation tests. -ss
Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop
On (20/07/29 22:28), Herbert Xu wrote: > This miniseries breaks a header loop involving qspinlock_types.h. > The issue is that qspinlock_types.h includes atomic.h, which then > eventually includes kernel.h which could lead back to the original > file via spinlock_types.h. > > The first patch moves ATOMIC_INIT into linux/types.h while the second > patch actuallys breaks the loop by no longer including atomic.h > in qspinlock_types.h. Thanks for staying on this. I pulled locking/header, run some cross-compilation tests locally (powerpc, arm, arm64, x86) and pushed printk/for-next. Let's see. -ss