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]
Date: Mon, 12 Feb 2024 16:24:44 +0530
From: Nilay Shroff <nilay@...ux.ibm.com>
To: john.g.garry@...cle.com
Cc: axboe@...nel.dk, brauner@...nel.org, bvanassche@....org,
        dchinner@...hat.com, djwong@...nel.org, hch@....de, jack@...e.cz,
        jbongio@...gle.com, jejb@...ux.ibm.com, kbusch@...nel.org,
        linux-block@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-nvme@...ts.infradead.org,
        linux-scsi@...r.kernel.org, linux-xfs@...r.kernel.org,
        martin.petersen@...cle.com, ming.lei@...hat.com, ojaswin@...ux.ibm.com,
        sagi@...mberg.me, tytso@....edu, viro@...iv.linux.org.uk
Subject: Re:[PATCH v3 09/15] block: Add checks to merging of atomic writes

>+static bool rq_straddles_atomic_write_boundary(struct request *rq,
>+					unsigned int front,
>+					unsigned int back)
>+{
>+	unsigned int boundary = queue_atomic_write_boundary_bytes(rq->q);
>+	unsigned int mask, imask;
>+	loff_t start, end;
>+
>+	if (!boundary)
>+		return false;
>+
>+	start = rq->__sector << SECTOR_SHIFT;
>+	end = start + rq->__data_len;
>+
>+	start -= front;
>+	end += back;
>+
>+	/* We're longer than the boundary, so must be crossing it */
>+	if (end - start > boundary)
>+		return true;
>+
>+	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;
>+
>+	return false;
>+}
>+

Shall we ensure here that we don't cross max limit of atomic write supported by 
device? It seems that if the boundary size is not advertized by the device 
(in fact, I have one NVMe drive which has boundary size zero i.e. nabo/nabspf/
nawupf are all zero but awupf is non-zero) then we (unconditionally) allow 
merging. However it may be possible that post merging the total size of the 
request may exceed the atomic-write-unit-max-size supported by the device and 
if that happens then most probably we would be able to catch it very late in 
the driver code (if the device is NVMe). 

So is it a good idea to validate here whether we could potentially exceed 
the atomic-write-max-unit-size supported by device before we allow merging? 
In case we exceed the atomic-write-max-unit-size post merge then don't allow
merging?
 
Thanks,
--Nilay


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ