Re: [PATCH 3/7] dm: handle failures in dm_table_set_restrictions

[Mikulas Patocka <mpatocka@redhat.com>]

On Fri, 14 Mar 2025, Mikulas Patocka wrote:

> 
> 
> On Tue, 11 Mar 2025, Damien Le Moal wrote:
> 
> > Yes, for simple scalar limits, I do not think there is any issue. But there are
> > some cases where changing one limit implies a change to other limits when the
> > limits are committed (under the limits lock). So my concern was that if the
> > above runs simultaneously with a queue limits commit, we may endup with the
> > limits struct copy grabbing part of the new limits and thus resulting in an
> > inconsistent limits struct. Not entirely sure that can actually happen though.
> > But given that queue_limits_commit_update() does:
> > 
> > 	q->limits = *lim;
> > 
> > and this code does:
> > 
> > 	old_limits = q->limits;
> > 
> > we may endup depending on how the compiler handles the struct copy ?
> 
> There is no guarantee that struct copy will update the structure fields 
> atomically.
> 
> On some CPUs, a "rep movsb" instruction may be used, which may be 
> optimized by the CPU, but it may be also interrupted at any byte boundary.
> 
> I think it should be changed to the sequence of WRITE_ONCE statements, for 
> example:
> WRITE_ONCE(q->limits->file, lim->field);
> 
> Mikulas

BTW. some SPARC CPUs had an instruction that would zero a cache line 
without loading it from memory. The memcpy implementation on SPARC used 
this instruction to avoid loading data that would be soon overwritten.

The result was that if you read a region of memory concurrently with 
memcpy writing to it, you could read zeros produced by this instruction.

Mikulas