Re: [PATCH] ksmbd: set fixed sector size to FS_SECTOR_SIZE_INFORMATION

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/12/2022 11:23 PM, Namjae Jeon wrote:
2022-04-13 10:40 GMT+09:00, Tom Talpey <tom@xxxxxxxxxx>:
On 4/12/2022 6:58 PM, Namjae Jeon wrote:
Currently ksmbd is using ->f_bsize from vfs_statfs() as sector size.
If fat/exfat is a local share, ->f_bsize is a cluster size that is too
large to be used as a sector size. Sector sizes larger than 4K cause
problem occurs when mounting an iso file through windows client.

The error message can be obtained using Mount-DiskImage command,
   the error is:
"Mount-DiskImage : The sector size of the physical disk on which the
virtual disk resides is not supported."

This patch reports fixed sector size as 512B logical/4K physical to
windows client to avoid poking into the block device.

Cc: Christoph Hellwig <hch@xxxxxx>
Signed-off-by: Namjae Jeon <linkinjeon@xxxxxxxxxx>
---
   fs/ksmbd/smb2pdu.c | 9 ++++-----
   1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 62cc0f95ab87..28ff7c058bc4 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -4998,12 +4998,11 @@ static int smb2_get_info_filesystem(struct
ksmbd_work *work,

   		info = (struct smb3_fs_ss_info *)(rsp->Buffer);

-		info->LogicalBytesPerSector = cpu_to_le32(stfs.f_bsize);
-		info->PhysicalBytesPerSectorForAtomicity =
-				cpu_to_le32(stfs.f_bsize);
-		info->PhysicalBytesPerSectorForPerf = cpu_to_le32(stfs.f_bsize);
+		info->LogicalBytesPerSector = cpu_to_le32(512);
+		info->PhysicalBytesPerSectorForAtomicity = cpu_to_le32(4096);
+		info->PhysicalBytesPerSectorForPerf = cpu_to_le32(4096);
   		info->FSEffPhysicalBytesPerSectorForAtomicity =
-				cpu_to_le32(stfs.f_bsize);
+				cpu_to_le32(4096);

Hi Tom,
So, this sounds like a great approach, much better than returning 128K.
Thanks:)

However, it's not at all universally true that 4K is going to be atomic.
Could you please elaborate more ? Is the atomic size not 4K for 4K
native storage?

Where is it guaranteed that the physical storage is 4K??

And min(stfs.f_bsize, 4096) might be problematic too. Is there any better
option??
There is no option than to poke into block layer, but Christoph
pointed out that this will also give the wrong answer for file systems
with multiple device support (btrfs, f2fs, xfs).

And I agree. Did you read the discussion in MS-FSCC by the way?

A client can interpret this field as the unit for which NTFS guarantees an atomic
operation. NTFS calculates the value of this field as follows:
 Retrieve the physical sector size the device reports for atomicity, and store in x.
 Validate that the value x is greater than or equal to the logical sector size. If it is not, set x to the
logical sector size.
 Validate that the value x is a power of two. If it is not, set x to the logical sector size.
 Validate that the value x is less than or equal to the system page size defined in [MS-FSA] section
2.1.1.1. If it is not, set x to the system page size defined in [MS-FSA] section 2.1.1.1.

That's just the Windows/NTFS approach, of course.

So we need to select fixed size as between 512B ~ 4KB. I think the v2
patch looks a bit better...

"A bit better"? So what's the actual fix going to be?

Tom.

Thanks!

Tom.


   		info->Flags = cpu_to_le32(SSINFO_FLAGS_ALIGNED_DEVICE |
   				    SSINFO_FLAGS_PARTITION_ALIGNED_ON_DEVICE);
   		info->ByteOffsetForSectorAlignment = 0;





[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux