From: Matthieu Baerts <matttbe@kernel.org>
To: Mat Martineau <martineau@kernel.org>
Cc: Geliang Tang <geliang@kernel.org>,
mptcp@lists.linux.dev, Geliang Tang <tanggeliang@kylinos.cn>
Subject: Re: [PATCH mptcp-next] Squash to "mptcp: add last time fields in mptcp_info"
Date: Sat, 6 Apr 2024 07:13:22 +0200 (GMT+02:00) [thread overview]
Message-ID: <dac737b6-ec30-4edf-a3f0-8471d93c54a1@kernel.org> (raw)
In-Reply-To: <5900b56b-6654-b7e0-2fd4-88abeaeb52c1@kernel.org>
Hi Mat,
6 Apr 2024 02:48:20 Mat Martineau <martineau@kernel.org>:
> On Sat, 6 Apr 2024, Geliang Tang wrote:
>
>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>
>> Reload now = jiffies32 as Eric suggested.
>>
>> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
>> ---
>> net/mptcp/sockopt.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
>> index 2ec2fdf9f4af..17223da5436d 100644
>> --- a/net/mptcp/sockopt.c
>> +++ b/net/mptcp/sockopt.c
>> @@ -944,6 +944,7 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
>> info->mptcpi_bytes_retrans = msk->bytes_retrans;
>> info->mptcpi_subflows_total = info->mptcpi_subflows +
>> __mptcp_has_initial_subflow(msk);
>> + now = tcp_jiffies32; /* reload it, lock_sock_fast can sleep and be quite slow */
>> info->mptcpi_last_data_sent = jiffies_to_msecs(now - msk->last_data_sent);
>> info->mptcpi_last_data_recv = jiffies_to_msecs(now - msk->last_data_recv);
>> unlock_sock_fast(sk, slow);
>
> Geliang and Matthieu -
>
> I'd like to minimize the time difference between the assigments for all the last time fields. For example, if the last_ack_recv is copied before the socket lock is acquired, the value in the info struct could become stale while waiting for the lock and not match up well with last_data_sent.
>
> Could instead move the entire mptcp_data_lock()/mptcp_data_unlock() block inside the socket lock critical section. Then, if 'now' is assigned with both locks acquired there should not be significant delays between them.
Yes, that's what I had in mind when I replied to Eric.
Because the data lock is a spin lock, maybe we don't need to reload it there if this block is moved after the sk lock? So all the counters would be synced.
Cheers,
Matt
next prev parent reply other threads:[~2024-04-06 5:13 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-06 0:30 [PATCH mptcp-next] Squash to "mptcp: add last time fields in mptcp_info" Geliang Tang
2024-04-06 0:48 ` Mat Martineau
2024-04-06 5:13 ` Matthieu Baerts [this message]
2024-04-06 1:19 ` MPTCP CI
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=dac737b6-ec30-4edf-a3f0-8471d93c54a1@kernel.org \
--to=matttbe@kernel.org \
--cc=geliang@kernel.org \
--cc=martineau@kernel.org \
--cc=mptcp@lists.linux.dev \
--cc=tanggeliang@kylinos.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).