Re: [PATCH v3 1/5] batman-adv: Do not send uninitialized TT changes

[Sven Eckelmann <sven@narfation.org>]

[-- Attachment #1: Type: text/plain, Size: 1399 bytes --]

On Thursday, 21 November 2024 21:24:31 CET Remi Pommarel wrote:
> >
> > And then you can also move this before "tt_diff_entries_num = ..." and 
> > save the corresponding bat_priv->tt.local_changes for the spliced list to the 
> > inside the lock also in a local variable. And then operate on this variable 
> > for the other decisions. Of course, you must still clean the local list in 
> > case of an error. Which of course would slightly change the behavior in case 
> > of an allocation error in batadv_tt_prepare_tvlv_local_data (which would 
> > previously kept the list as it was).
> >
> > But if it would be done like this then we could also remove the READ_ONCE and 
> > not introduce the WRITE_ONCE - just because local_changes is only touched
> > inside a locked area (see changes_list_lock).
> >
> > Please double check these statements - this was just a simple brain dump.
> 
> Yes that would be a much more elegant way to handle it. Unfortunately,
> if I don't miss anything, the WRITE_ONCE/READ_ONCE would still be
> needed because batadv_tt_local_commit_changes_nolock() has to load
> tt.local_changes out of the lock to check if it needs to purge client
> and recompute CRCs.

Ah, you are right. I've missed this one.

Btw. just to make it clear: These changes wouldn't be for this patch/fix 
anyway. Just for a potential refactoring/cleanup patch for net-next.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]