linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <htejun@gmail.com>
To: Jeff Garzik <jeff@garzik.org>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>, linux-ide@vger.kernel.org
Subject: Re: [PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long
Date: Wed, 07 Nov 2007 10:12:25 +0900	[thread overview]
Message-ID: <473110F9.6020404@gmail.com> (raw)
In-Reply-To: <4730AA0A.4090904@garzik.org>

Jeff Garzik wrote:
>> 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

I don't really see much point in this.  What's the advantage of a
machine int for xfer mask?

> * it is 32-bit on 32-bit architectures

The problem I see for using native integral types for flags or mask is
that it's not fixed in size, so you can only use the smallest of all the
supported architectures.  We know for all archs we support, both
unsigned int and unsigned long are at least 32bits long but to me making
the assumption about expected number of bits using u32 or u64 is much
more important than other considerations.

> * 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

I don't think it's likely that we'll need those heavy machineries for
xfermask and the correct approach is to convert when the need actually
arises.

> * 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.

I'm not trying to optimize performance or size.  I'm trying to make
programming assumptions clear.  I think it's silly to optimize anything
for xfermask.  It just has to work and be clear to people working with it.

An optimal but overkill solution would be using fast_u32 or fast_u64
types such that the compiler can choose as necessary but as we don't
have that yet && xfermask handling is a very very cold path, I think we
should be aiming for clarity.

Thanks.

-- 
tejun

  reply	other threads:[~2007-11-07  1:12 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-06  5:38 [PATCHSET] libata: update timing and fix pata_amd transfer mode selection Tejun Heo
2007-11-06  5:38 ` [PATCH 01/12] ata_generic: unindent loop in generic_set_mode() Tejun Heo
2007-11-06  5:39 ` [PATCH 02/12] libata: export xfermode / PATA timing related functions Tejun Heo
2007-11-06  5:39 ` [PATCH 03/12] libata: clean up xfermode / PATA timing related stuff Tejun Heo
2007-11-06  5:39 ` [PATCH 04/12] libata: kill ata_id_to_dma_mode() Tejun Heo
2007-11-06 10:49   ` Alan Cox
2007-11-06 11:21     ` Tejun Heo
2007-11-24  1:13   ` Jeff Garzik
2007-11-06  5:39 ` [PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long Tejun Heo
2007-11-06 10:51   ` Alan Cox
2007-11-06 10:59     ` Tejun Heo
2007-11-06 17:53       ` Jeff Garzik
2007-11-07  1:12         ` Tejun Heo [this message]
2007-11-24  1:13   ` Jeff Garzik
2007-11-24  1:13     ` Jeff Garzik
2007-11-27  8:42       ` Tejun Heo
2007-11-06  5:39 ` [PATCH 06/12] libata: separate out ata_acpi_gtm_xfermask() from pacpi_discover_modes() Tejun Heo
2007-11-06 10:54   ` Alan Cox
2007-11-06 11:00     ` Tejun Heo
2007-11-24  1:14   ` Jeff Garzik
2007-11-06  5:39 ` [PATCH 07/12] libata: fix ata_acpi_gtm_xfermask() Tejun Heo
2007-11-06 10:55   ` Alan Cox
2007-11-24  1:16   ` Jeff Garzik
2007-11-27  8:40     ` Tejun Heo
2007-11-06  5:39 ` [PATCH 08/12] libata: implement ata_timing_cycle2mode() and use it in libata-acpi and pata_acpi Tejun Heo
2007-11-06 10:59   ` Alan Cox
2007-11-06 11:09     ` Tejun Heo
2007-11-24  1:17   ` Jeff Garzik
2007-11-06  5:39 ` [PATCH 09/12] libata: implement ata_acpi_init_gtm() Tejun Heo
2007-11-06  5:39 ` [PATCH 10/12] libata: reimplement ata_acpi_cbl_80wire() using ata_acpi_gtm_xfermask() Tejun Heo
2007-11-06  5:39 ` [PATCH 11/12] libata: add ATA_CBL_PATA_IGN Tejun Heo
2007-11-06 10:59   ` Alan Cox
2007-11-06 11:02     ` Tejun Heo
2007-11-06 11:25       ` Alan Cox
2007-11-06  5:39 ` [PATCH 12/12] pata_amd: update mode selection for NV PATAs Tejun Heo
2007-11-06 10:59   ` Alan Cox
2007-11-23  1:08 ` [PATCHSET] libata: update timing and fix pata_amd transfer mode selection Tejun Heo
2007-11-24  1:18 ` Jeff Garzik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=473110F9.6020404@gmail.com \
    --to=htejun@gmail.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=jeff@garzik.org \
    --cc=linux-ide@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).