linux-kernel - Re: [PATCH v4 05/11] block: Add core atomic write support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <87il2c20q9.fsf@doe.com>
Date: Sun, 25 Feb 2024 17:51:34 +0530
From: Ritesh Harjani (IBM) <ritesh.list@...il.com>
To: John Garry <john.g.garry@...cle.com>, axboe@...nel.dk, kbusch@...nel.org, hch@....de, sagi@...mberg.me, jejb@...ux.ibm.com, martin.petersen@...cle.com, djwong@...nel.org, viro@...iv.linux.org.uk, brauner@...nel.org, dchinner@...hat.com, jack@...e.cz
Cc: linux-block@...r.kernel.org, linux-kernel@...r.kernel.org, linux-nvme@...ts.infradead.org, linux-fsdevel@...r.kernel.org, tytso@....edu, jbongio@...gle.com, linux-scsi@...r.kernel.org, ojaswin@...ux.ibm.com, linux-aio@...ck.org, linux-btrfs@...r.kernel.org, io-uring@...r.kernel.org, nilay@...ux.ibm.com, John Garry <john.g.garry@...cle.com>
Subject: Re: [PATCH v4 05/11] block: Add core atomic write support

Ritesh Harjani (IBM) <ritesh.list@...il.com> writes:

> John Garry <john.g.garry@...cle.com> writes:
>
>> +
>> +	mask = boundary - 1;
>> +
>> +	/* start/end are boundary-aligned, so cannot be crossing */
>> +	if (!(start & mask) || !(end & mask))
>> +		return false;
>> +
>> +	imask = ~mask;
>> +
>> +	/* Top bits are different, so crossed a boundary */
>> +	if ((start & imask) != (end & imask))
>> +		return true;
>
> The last condition looks wrong. Shouldn't it be end - 1?
>
>> +
>> +	return false;
>> +}
>
> Can we do something like this?
>
> static bool rq_straddles_atomic_write_boundary(struct request *rq,
> 					       unsigned int start_adjust,
> 					       unsigned int end_adjust)
> {
> 	unsigned int boundary = queue_atomic_write_boundary_bytes(rq->q);
> 	unsigned long boundary_mask;
> 	unsigned long start_rq_pos, end_rq_pos;
>
> 	if (!boundary)
> 		return false;
>
> 	start_rq_pos = blk_rq_pos(rq) << SECTOR_SHIFT;
> 	end_rq_pos = start_rq_pos + blk_rq_bytes(rq);

my bad. I meant this...

   end_rq_pos = start_rq_pos + blk_rq_bytes(rq) - 1;
>
> 	start_rq_pos -= start_adjust;
> 	end_rq_pos += end_adjust;
>
> 	boundary_mask = boundary - 1;
>
> 	if ((start_rq_pos | boundary_mask) != (end_rq_pos | boundary_mask))
> 		return true;
>
> 	return false;
> }
>
> I was thinking this check should cover all cases? Thoughts?
>
>

-ritesh