* The value of FB_MTU eats two pages
@ 2021-06-01 14:18 Menglong Dong
2021-06-02 19:50 ` Jon Maloy
0 siblings, 1 reply; 5+ messages in thread
From: Menglong Dong @ 2021-06-01 14:18 UTC (permalink / raw)
To: jmaloy
Cc: ying.xue, David Miller, Jakub Kicinski, etdev, tipc-discussion, LKML
Hello!
I have a question about the value of FB_MTU in tipc, how does the '3744' form?
I notice that it is used in 'tipc_msg_build()' when memory allocation
fails, and it
tries to fall back to a smaller MTU to avoid unnecessary sending failures.
However, the size of the data allocated will be more than 4096 when FB_MTU
is 3744. I did a rough calculation, the size of data will more than 4200:
(FB_MTU + TIPCHDR + BUF_HEADROOM + sizeof(struct skb_shared_info))
Therefore, 8192 will be allocated from slab, and about 4000 of it will
not be used.
FB_MTU is used for low memory, and I think eating two pages will make it worse.
Do I miss something?
Thanks!
Menglong Dong
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: The value of FB_MTU eats two pages
2021-06-01 14:18 The value of FB_MTU eats two pages Menglong Dong
@ 2021-06-02 19:50 ` Jon Maloy
2021-06-03 2:26 ` Menglong Dong
0 siblings, 1 reply; 5+ messages in thread
From: Jon Maloy @ 2021-06-02 19:50 UTC (permalink / raw)
To: Menglong Dong
Cc: ying.xue, David Miller, Jakub Kicinski, etdev, tipc-discussion, LKML
On 6/1/21 10:18 AM, Menglong Dong wrote:
> Hello!
>
> I have a question about the value of FB_MTU in tipc, how does the '3744' form?
> I notice that it is used in 'tipc_msg_build()' when memory allocation
> fails, and it
> tries to fall back to a smaller MTU to avoid unnecessary sending failures.
>
> However, the size of the data allocated will be more than 4096 when FB_MTU
> is 3744. I did a rough calculation, the size of data will more than 4200:
>
> (FB_MTU + TIPCHDR + BUF_HEADROOM + sizeof(struct skb_shared_info))
>
> Therefore, 8192 will be allocated from slab, and about 4000 of it will
> not be used.
>
> FB_MTU is used for low memory, and I think eating two pages will make it worse.
> Do I miss something?
>
> Thanks!
> Menglong Dong
>
Hi Dong,
The value is based on empiric knowledge.
When I determined it I made a small loop in a kernel driver where I
allocated skbs (using tipc_buf_acquire) with an increasing size
(incremented with 1 each iteration), and then printed out the
corresponding truesize.
That gave the value we are using now.
Now, when re-running the test I get a different value, so something has
obviously changed since then.
[ 1622.158586] skb(513) =>> truesize 2304, prev skb(512) => prev
truesize 1280
[ 1622.162074] skb(1537) =>> truesize 4352, prev skb(1536) => prev
truesize 2304
[ 1622.165984] skb(3585) =>> truesize 8448, prev skb(3584) => prev
truesize 4352
As you can see, the optimal value now, for an x86_64 machine compiled
with gcc, is 3584 bytes, not 3744.
Feel free to post a patch for this if you want to.
Thanks
///Jon Maloy
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: The value of FB_MTU eats two pages
2021-06-02 19:50 ` Jon Maloy
@ 2021-06-03 2:26 ` Menglong Dong
2021-06-03 13:08 ` Jon Maloy
0 siblings, 1 reply; 5+ messages in thread
From: Menglong Dong @ 2021-06-03 2:26 UTC (permalink / raw)
To: Jon Maloy
Cc: ying.xue, David Miller, Jakub Kicinski, tipc-discussion, LKML, netdev
Hello Maloy,
On Thu, Jun 3, 2021 at 3:50 AM Jon Maloy <jmaloy@redhat.com> wrote:
[...]
> Hi Dong,
> The value is based on empiric knowledge.
> When I determined it I made a small loop in a kernel driver where I
> allocated skbs (using tipc_buf_acquire) with an increasing size
> (incremented with 1 each iteration), and then printed out the
> corresponding truesize.
>
> That gave the value we are using now.
>
> Now, when re-running the test I get a different value, so something has
> obviously changed since then.
>
> [ 1622.158586] skb(513) =>> truesize 2304, prev skb(512) => prev
> truesize 1280
> [ 1622.162074] skb(1537) =>> truesize 4352, prev skb(1536) => prev
> truesize 2304
> [ 1622.165984] skb(3585) =>> truesize 8448, prev skb(3584) => prev
> truesize 4352
>
> As you can see, the optimal value now, for an x86_64 machine compiled
> with gcc, is 3584 bytes, not 3744.
I'm not sure if this is a perfect way to determine the value of FB_MTU.
If 'struct skb_shared_info' changes, this value seems should change,
too.
How about we make it this:
#define FB_MTU (PAGE_SIZE - \
SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) - \
SKB_DATA_ALIGN(BUF_HEADROOM + BUF_TAILROOM + 3 + \
MAX_H_SIZ))
The value 'BUF_HEADROOM + BUF_TAILROOM + 3' come from 'tipc_buf_acquire()':
#ifdef CONFIG_TIPC_CRYPTO
unsigned int buf_size = (BUF_HEADROOM + size + BUF_TAILROOM + 3) & ~3u;
#else
unsigned int buf_size = (BUF_HEADROOM + size + 3) & ~3u;
#endif
Is it a good idea?
Thanks
Menglong Dong
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: The value of FB_MTU eats two pages
2021-06-03 2:26 ` Menglong Dong
@ 2021-06-03 13:08 ` Jon Maloy
2021-06-03 14:54 ` Menglong Dong
0 siblings, 1 reply; 5+ messages in thread
From: Jon Maloy @ 2021-06-03 13:08 UTC (permalink / raw)
To: Menglong Dong
Cc: ying.xue, David Miller, Jakub Kicinski, tipc-discussion, LKML, netdev
On 6/2/21 10:26 PM, Menglong Dong wrote:
> Hello Maloy,
>
> On Thu, Jun 3, 2021 at 3:50 AM Jon Maloy <jmaloy@redhat.com> wrote:
>
> [...]
>> Hi Dong,
>> The value is based on empiric knowledge.
>> When I determined it I made a small loop in a kernel driver where I
>> allocated skbs (using tipc_buf_acquire) with an increasing size
>> (incremented with 1 each iteration), and then printed out the
>> corresponding truesize.
>>
>> That gave the value we are using now.
>>
>> Now, when re-running the test I get a different value, so something has
>> obviously changed since then.
>>
>> [ 1622.158586] skb(513) =>> truesize 2304, prev skb(512) => prev
>> truesize 1280
>> [ 1622.162074] skb(1537) =>> truesize 4352, prev skb(1536) => prev
>> truesize 2304
>> [ 1622.165984] skb(3585) =>> truesize 8448, prev skb(3584) => prev
>> truesize 4352
>>
>> As you can see, the optimal value now, for an x86_64 machine compiled
>> with gcc, is 3584 bytes, not 3744.
> I'm not sure if this is a perfect way to determine the value of FB_MTU.
> If 'struct skb_shared_info' changes, this value seems should change,
> too.
>
> How about we make it this:
>
> #define FB_MTU (PAGE_SIZE - \
> SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) - \
> SKB_DATA_ALIGN(BUF_HEADROOM + BUF_TAILROOM + 3 + \
> MAX_H_SIZ))
>
> The value 'BUF_HEADROOM + BUF_TAILROOM + 3' come from 'tipc_buf_acquire()':
>
> #ifdef CONFIG_TIPC_CRYPTO
> unsigned int buf_size = (BUF_HEADROOM + size + BUF_TAILROOM + 3) & ~3u;
> #else
> unsigned int buf_size = (BUF_HEADROOM + size + 3) & ~3u;
> #endif
>
> Is it a good idea?
Yes, I think that makes sense. I was always aware of the "fragility" of
my approach, -this one looks more future safe.
///jon
>
> Thanks
> Menglong Dong
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: The value of FB_MTU eats two pages
2021-06-03 13:08 ` Jon Maloy
@ 2021-06-03 14:54 ` Menglong Dong
0 siblings, 0 replies; 5+ messages in thread
From: Menglong Dong @ 2021-06-03 14:54 UTC (permalink / raw)
To: Jon Maloy
Cc: ying.xue, David Miller, Jakub Kicinski, tipc-discussion, LKML, netdev
On Thu, Jun 3, 2021 at 9:08 PM Jon Maloy <jmaloy@redhat.com> wrote:
>
[...]
> Yes, I think that makes sense. I was always aware of the "fragility" of
> my approach, -this one looks more future safe.
>
Ok, I will post a patch.
Menglong Dong
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-06-03 14:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-01 14:18 The value of FB_MTU eats two pages Menglong Dong
2021-06-02 19:50 ` Jon Maloy
2021-06-03 2:26 ` Menglong Dong
2021-06-03 13:08 ` Jon Maloy
2021-06-03 14:54 ` Menglong Dong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).