Re: [PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long

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

 



Tejun Heo wrote:
Alan Cox wrote:
On Tue,  6 Nov 2007 14:39:03 +0900
Tejun Heo <htejun@xxxxxxxxx> wrote:

xfer_mask is unsigned int not unsigned long.  Change ->mode_filter to
take and return unsigned int.

While at it, rename @adev of ata_pci_default_filter() to @dev for
consistency.

Signed-off-by: Tejun Heo <htejun@xxxxxxxxx>
The filter type was purposefully unsigned long to allow for expansion (eg
for SWDMA) without breaking drivers. No problem with it changing but I'd
say "unsigned int" was the worst possible choice - its now shorter (no
room for expansion) and size dependant on arch. u32 would be shorter and
consistent across all platforms, ulong would have more room for expansion.

I agree it should be one of u* types and am happy to convert.  This
patch is primarily for consistency as in libata-core, xfer_mask is
unsigned int.  My first proposal was u32 too but Jeff vetoed it.  IIRC,
Jeff has affection for machine-independent integral types.  :-)

Anyways, I think ulong is worse than uint because ulong differs in size
between 32 and 64 archs.  What's the good of extra 32bits in flags space
if it's not there on 32bit archs?  Also, we currently only use 20bits of
xfermask, 12 extra bits seem enough for the time, especially as
everything is moving over to SATA where xfermode doesn't really matter, no?

Jeff, are you okay with u32 or 64?

unsigned long is IMO the best choice for bitmaps.

* it is a machine int on all(?) architectures

* it is 32-bit on 32-bit architectures

* it is consistent with test_bit(), set_bit() and lib/bitmap.c interfaces

* conversion to test_bit() and lib/bitmap.c interfaces as a future step is trivial

* your structs inflate on 64-bit due to pointers anyway, so I see little real consequence to using the lower 32 bits of an unsigned long as a portable meme.

	Jeff


-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux