Prev: libata: take advantage of cmwq and remove concurrency limitations
Next: async: use workqueue for worker pool
From: Tejun Heo on 29 Jun 2010 11:50
Hello,

On 06/29/2010 02:36 PM, Frederic Weisbecker wrote:
> But then, if each cpu pointers are aligned, the struct itself doesn't need
> to be aligned in its size right? It would need to if multiple elements
> were allocated per cpu but for this struct we only have one per cpu. So
> what seems to matter wrt alignment is only the base pointer of these structs,
> not the size.

Yeap, sure, but how does it matter?

Thanks.

--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Tejun Heo on 29 Jun 2010 12:00
Hello, Frederic.

On 06/29/2010 05:47 PM, Frederic Weisbecker wrote:
> It means the size of the struct itself doesn't need to aligned to anything.

Yeah, it could be that I'm kinda dense today but your mode of
communication is a bit too implicit for me. So, can you please
elaborate what problem you see in the patch and how you want it to be
changed?

Thanks.

--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Frederic Weisbecker on 29 Jun 2010 12:10
On Tue, Jun 29, 2010 at 05:51:14PM +0200, Tejun Heo wrote:
> Hello, Frederic.
>
> On 06/29/2010 05:47 PM, Frederic Weisbecker wrote:
> > It means the size of the struct itself doesn't need to aligned to anything.
>
> Yeah, it could be that I'm kinda dense today but your mode of
> communication is a bit too implicit for me. So, can you please
> elaborate what problem you see in the patch and how you want it to be
> changed?
>
> Thanks.


So, imagine you allocate your struct with alloc_percpu(align).

The per cpu pointer is 0x400 (purely imagination).

Now you have two cpus and they have the following base offsets for
per cpu allocations:

CPU 0 = 0xf1000000
CPU 1 = 0xf2000000


So, the true pointers for your cpu workqueue structs will be:

CPU 0 = 0xf1000400
CPU 1 = 0xf2000400


These addresses are aligned like you wanted to, and it seems it is what
matters, to store these addresses in the work flags.

So why does the size of the struct need to be aligned too? All you want
is that the two above addresses are aligned. Now why the size of the struct
itself needs this alignment too. That's the obscure point for me. If it's
useless, this could avoid all this alignment maintainance, except during
the allocation itself.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Tejun Heo on 29 Jun 2010 12:20
Hello,

On 06/29/2010 06:01 PM, Frederic Weisbecker wrote:
> So, imagine you allocate your struct with alloc_percpu(align).
>
> The per cpu pointer is 0x400 (purely imagination).
>
> Now you have two cpus and they have the following base offsets for
> per cpu allocations:
>
> CPU 0 = 0xf1000000
> CPU 1 = 0xf2000000
>
> So, the true pointers for your cpu workqueue structs will be:
>
> CPU 0 = 0xf1000400
> CPU 1 = 0xf2000400
>
> These addresses are aligned like you wanted to, and it seems it is what
> matters, to store these addresses in the work flags.

Yes.

> So why does the size of the struct need to be aligned too?

Where am I doing that?

> All you want is that the two above addresses are aligned. Now why
> the size of the struct itself needs this alignment too. That's the
> obscure point for me. If it's useless, this could avoid all this
> alignment maintainance, except during the allocation itself.

What alignment maintenance? Are you talking about the UP code? If
you're talking about the UP code, the ugliness there is because the
current UP __alloc_percpu() can't honor the alignment parameter.

Heh, it seems I'm still lost. Care to give one more shot at it? :-)

Thanks.

--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Frederic Weisbecker on 29 Jun 2010 12:20
On Tue, Jun 29, 2010 at 06:09:35PM +0200, Tejun Heo wrote:
> Hello,
>
> On 06/29/2010 06:01 PM, Frederic Weisbecker wrote:
> > So, imagine you allocate your struct with alloc_percpu(align).
> >
> > The per cpu pointer is 0x400 (purely imagination).
> >
> > Now you have two cpus and they have the following base offsets for
> > per cpu allocations:
> >
> > CPU 0 = 0xf1000000
> > CPU 1 = 0xf2000000
> >
> > So, the true pointers for your cpu workqueue structs will be:
> >
> > CPU 0 = 0xf1000400
> > CPU 1 = 0xf2000400
> >
> > These addresses are aligned like you wanted to, and it seems it is what
> > matters, to store these addresses in the work flags.
>
> Yes.
>
> > So why does the size of the struct need to be aligned too?
>
> Where am I doing that?


Ah well, yesterday there was a BUILD_BUG_ON on init_workqueues that checked
this structure size was well aligned. Now that I check again, it seems to have
disappeared after you updated the patch.



>
> > All you want is that the two above addresses are aligned. Now why
> > the size of the struct itself needs this alignment too. That's the
> > obscure point for me. If it's useless, this could avoid all this
> > alignment maintainance, except during the allocation itself.
>
> What alignment maintenance? Are you talking about the UP code? If
> you're talking about the UP code, the ugliness there is because the
> current UP __alloc_percpu() can't honor the alignment parameter.


Heh no, it's about a leftover in your patchset that you have fixed
now.


> Heh, it seems I'm still lost. Care to give one more shot at it? :-)


My bad, I haven't looked your updated patch in detail... :)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
First  |  Prev  |  Next  |  Last
Pages: 1 2 3
Prev: libata: take advantage of cmwq and remove concurrency limitations
Next: async: use workqueue for worker pool