public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jeff Garzik <jgarzik@pobox.com>
To: Tejun Heo <htejun@gmail.com>
Cc: axboe@suse.de, James.Bottomley@steeleye.com, bzolnier@gmail.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH Linux 2.6.12-rc5-mm2 07/09] blk: update libata to use the new blk_ordered.
Date: Sun, 05 Jun 2005 03:02:51 -0400	[thread overview]
Message-ID: <42A2A39B.5020103@pobox.com> (raw)
In-Reply-To: <20050605055337.13444DD8@htj.dyndns.org>

Tejun Heo wrote:
> 07_blk_update_libata_to_use_new_ordered.patch
> 
> 	Reflect changes in SCSI midlayer and updated to use the new
> 	ordered request implementation.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

I would prefer separate patches for:

* implement support for FUA bit in libata SCSI simulator

* update libata for your ordered flush changes



> Index: blk-fixes/drivers/scsi/ahci.c
> ===================================================================
> --- blk-fixes.orig/drivers/scsi/ahci.c	2005-06-05 14:50:11.000000000 +0900
> +++ blk-fixes/drivers/scsi/ahci.c	2005-06-05 14:53:35.000000000 +0900
> @@ -203,7 +203,6 @@ static Scsi_Host_Template ahci_sht = {
>  	.dma_boundary		= AHCI_DMA_BOUNDARY,
>  	.slave_configure	= ata_scsi_slave_config,
>  	.bios_param		= ata_std_bios_param,
> -	.ordered_flush		= 1,
>  };
>  
>  static struct ata_port_operations ahci_ops = {
> Index: blk-fixes/drivers/scsi/ata_piix.c
> ===================================================================
> --- blk-fixes.orig/drivers/scsi/ata_piix.c	2005-06-05 14:50:11.000000000 +0900
> +++ blk-fixes/drivers/scsi/ata_piix.c	2005-06-05 14:53:35.000000000 +0900
> @@ -123,7 +123,6 @@ static Scsi_Host_Template piix_sht = {
>  	.dma_boundary		= ATA_DMA_BOUNDARY,
>  	.slave_configure	= ata_scsi_slave_config,
>  	.bios_param		= ata_std_bios_param,
> -	.ordered_flush		= 1,
>  };
>  
>  static struct ata_port_operations piix_pata_ops = {
> Index: blk-fixes/drivers/scsi/sata_nv.c
> ===================================================================
> --- blk-fixes.orig/drivers/scsi/sata_nv.c	2005-06-05 14:50:11.000000000 +0900
> +++ blk-fixes/drivers/scsi/sata_nv.c	2005-06-05 14:53:35.000000000 +0900
> @@ -206,7 +206,6 @@ static Scsi_Host_Template nv_sht = {
>  	.dma_boundary		= ATA_DMA_BOUNDARY,
>  	.slave_configure	= ata_scsi_slave_config,
>  	.bios_param		= ata_std_bios_param,
> -	.ordered_flush		= 1,
>  };
>  
>  static struct ata_port_operations nv_ops = {
> Index: blk-fixes/drivers/scsi/sata_promise.c
> ===================================================================
> --- blk-fixes.orig/drivers/scsi/sata_promise.c	2005-06-05 14:50:11.000000000 +0900
> +++ blk-fixes/drivers/scsi/sata_promise.c	2005-06-05 14:53:35.000000000 +0900
> @@ -104,7 +104,6 @@ static Scsi_Host_Template pdc_ata_sht = 
>  	.dma_boundary		= ATA_DMA_BOUNDARY,
>  	.slave_configure	= ata_scsi_slave_config,
>  	.bios_param		= ata_std_bios_param,
> -	.ordered_flush		= 1,
>  };
>  
>  static struct ata_port_operations pdc_ata_ops = {
> Index: blk-fixes/drivers/scsi/sata_sil.c
> ===================================================================
> --- blk-fixes.orig/drivers/scsi/sata_sil.c	2005-06-05 14:50:11.000000000 +0900
> +++ blk-fixes/drivers/scsi/sata_sil.c	2005-06-05 14:53:35.000000000 +0900
> @@ -135,7 +135,6 @@ static Scsi_Host_Template sil_sht = {
>  	.dma_boundary		= ATA_DMA_BOUNDARY,
>  	.slave_configure	= ata_scsi_slave_config,
>  	.bios_param		= ata_std_bios_param,
> -	.ordered_flush		= 1,
>  };
>  
>  static struct ata_port_operations sil_ops = {
> Index: blk-fixes/drivers/scsi/sata_sis.c
> ===================================================================
> --- blk-fixes.orig/drivers/scsi/sata_sis.c	2005-06-05 14:50:11.000000000 +0900
> +++ blk-fixes/drivers/scsi/sata_sis.c	2005-06-05 14:53:35.000000000 +0900
> @@ -90,7 +90,6 @@ static Scsi_Host_Template sis_sht = {
>  	.dma_boundary		= ATA_DMA_BOUNDARY,
>  	.slave_configure	= ata_scsi_slave_config,
>  	.bios_param		= ata_std_bios_param,
> -	.ordered_flush		= 1,
>  };
>  
>  static struct ata_port_operations sis_ops = {
> Index: blk-fixes/drivers/scsi/sata_svw.c
> ===================================================================
> --- blk-fixes.orig/drivers/scsi/sata_svw.c	2005-06-05 14:50:11.000000000 +0900
> +++ blk-fixes/drivers/scsi/sata_svw.c	2005-06-05 14:53:35.000000000 +0900
> @@ -288,7 +288,6 @@ static Scsi_Host_Template k2_sata_sht = 
>  	.proc_info		= k2_sata_proc_info,
>  #endif
>  	.bios_param		= ata_std_bios_param,
> -	.ordered_flush		= 1,
>  };
>  
>  
> Index: blk-fixes/drivers/scsi/sata_sx4.c
> ===================================================================
> --- blk-fixes.orig/drivers/scsi/sata_sx4.c	2005-06-05 14:50:11.000000000 +0900
> +++ blk-fixes/drivers/scsi/sata_sx4.c	2005-06-05 14:53:35.000000000 +0900
> @@ -188,7 +188,6 @@ static Scsi_Host_Template pdc_sata_sht =
>  	.dma_boundary		= ATA_DMA_BOUNDARY,
>  	.slave_configure	= ata_scsi_slave_config,
>  	.bios_param		= ata_std_bios_param,
> -	.ordered_flush		= 1,
>  };
>  
>  static struct ata_port_operations pdc_20621_ops = {
> Index: blk-fixes/drivers/scsi/sata_uli.c
> ===================================================================
> --- blk-fixes.orig/drivers/scsi/sata_uli.c	2005-06-05 14:50:11.000000000 +0900
> +++ blk-fixes/drivers/scsi/sata_uli.c	2005-06-05 14:53:35.000000000 +0900
> @@ -82,7 +82,6 @@ static Scsi_Host_Template uli_sht = {
>  	.dma_boundary		= ATA_DMA_BOUNDARY,
>  	.slave_configure	= ata_scsi_slave_config,
>  	.bios_param		= ata_std_bios_param,
> -	.ordered_flush		= 1,
>  };
>  
>  static struct ata_port_operations uli_ops = {
> Index: blk-fixes/drivers/scsi/sata_via.c
> ===================================================================
> --- blk-fixes.orig/drivers/scsi/sata_via.c	2005-06-05 14:50:11.000000000 +0900
> +++ blk-fixes/drivers/scsi/sata_via.c	2005-06-05 14:53:35.000000000 +0900
> @@ -102,7 +102,6 @@ static Scsi_Host_Template svia_sht = {
>  	.dma_boundary		= ATA_DMA_BOUNDARY,
>  	.slave_configure	= ata_scsi_slave_config,
>  	.bios_param		= ata_std_bios_param,
> -	.ordered_flush		= 1,
>  };
>  
>  static struct ata_port_operations svia_sata_ops = {
> Index: blk-fixes/drivers/scsi/sata_vsc.c
> ===================================================================
> --- blk-fixes.orig/drivers/scsi/sata_vsc.c	2005-06-05 14:50:11.000000000 +0900
> +++ blk-fixes/drivers/scsi/sata_vsc.c	2005-06-05 14:53:35.000000000 +0900
> @@ -206,7 +206,6 @@ static Scsi_Host_Template vsc_sata_sht =
>  	.dma_boundary		= ATA_DMA_BOUNDARY,
>  	.slave_configure	= ata_scsi_slave_config,
>  	.bios_param		= ata_std_bios_param,
> -	.ordered_flush		= 1,
>  };
>  
>  
> Index: blk-fixes/drivers/scsi/libata-core.c
> ===================================================================
> --- blk-fixes.orig/drivers/scsi/libata-core.c	2005-06-05 14:50:11.000000000 +0900
> +++ blk-fixes/drivers/scsi/libata-core.c	2005-06-05 14:53:35.000000000 +0900
> @@ -510,19 +510,21 @@ void ata_tf_from_fis(u8 *fis, struct ata
>  }
>  
>  /**
> - *	ata_prot_to_cmd - determine which read/write opcodes to use
> + *	ata_prot_to_cmd - determine which read/write/fua-write opcodes to use
>   *	@protocol: ATA_PROT_xxx taskfile protocol
>   *	@lba48: true is lba48 is present
>   *
> - *	Given necessary input, determine which read/write commands
> - *	to use to transfer data.
> + *	Given necessary input, determine which read/write/fua-write
> + *	commands to use to transfer data.  Note that we only support
> + *	fua-writes on DMA LBA48 protocol.  In other cases, we simply
> + *	return 0 which is NOP.
>   *
>   *	LOCKING:
>   *	None.
>   */
>  static int ata_prot_to_cmd(int protocol, int lba48)
>  {
> -	int rcmd = 0, wcmd = 0;
> +	int rcmd = 0, wcmd = 0, wfua = 0;
>  
>  	switch (protocol) {
>  	case ATA_PROT_PIO:
> @@ -539,6 +541,7 @@ static int ata_prot_to_cmd(int protocol,
>  		if (lba48) {
>  			rcmd = ATA_CMD_READ_EXT;
>  			wcmd = ATA_CMD_WRITE_EXT;
> +			wfua = ATA_CMD_WRITE_FUA_EXT;
>  		} else {
>  			rcmd = ATA_CMD_READ;
>  			wcmd = ATA_CMD_WRITE;
> @@ -549,7 +552,7 @@ static int ata_prot_to_cmd(int protocol,
>  		return -1;
>  	}
>  
> -	return rcmd | (wcmd << 8);
> +	return rcmd | (wcmd << 8) | (wfua << 16);
>  }
>  
>  /**
> @@ -582,6 +585,7 @@ static void ata_dev_set_protocol(struct 
>  
>  	dev->read_cmd = cmd & 0xff;
>  	dev->write_cmd = (cmd >> 8) & 0xff;
> +	dev->write_fua_cmd = (cmd >> 16) & 0xff;
>  }
>  
>  static const char * xfer_mode_str[] = {
> Index: blk-fixes/drivers/scsi/libata-scsi.c
> ===================================================================
> --- blk-fixes.orig/drivers/scsi/libata-scsi.c	2005-06-05 14:50:11.000000000 +0900
> +++ blk-fixes/drivers/scsi/libata-scsi.c	2005-06-05 14:53:35.000000000 +0900
> @@ -569,6 +569,7 @@ static unsigned int ata_scsi_rw_xlat(str
>  	struct ata_device *dev = qc->dev;
>  	unsigned int lba   = tf->flags & ATA_TFLAG_LBA;
>  	unsigned int lba48 = tf->flags & ATA_TFLAG_LBA48;
> +	int fua = scsicmd[1] & 0x8;
>  	u64 block = 0;
>  	u32 n_block = 0;
>  
> @@ -577,9 +578,26 @@ static unsigned int ata_scsi_rw_xlat(str
>  
>  	if (scsicmd[0] == READ_10 || scsicmd[0] == READ_6 ||
>  	    scsicmd[0] == READ_16) {
> +		if (fua) {
> +			printk(KERN_WARNING
> +			       "ata%u(%u): WARNING: FUA READ unsupported\n",
> +			       qc->ap->id, qc->dev->devno);
> +			return 1;
> +		}
>  		tf->command = qc->dev->read_cmd;
>  	} else {
> -		tf->command = qc->dev->write_cmd;
> +		if (fua) {
> +			if (qc->dev->write_fua_cmd == 0 || !lba48) {
> +				printk(KERN_WARNING
> +				       "ata%u(%u): WARNING: FUA WRITE "
> +				       "unsupported with the current "
> +				       "protocol/addressing\n",
> +				       qc->ap->id, qc->dev->devno);
> +				return 1;
> +			}
> +			tf->command = qc->dev->write_fua_cmd;
> +		} else
> +			tf->command = qc->dev->write_cmd;
>  		tf->flags |= ATA_TFLAG_WRITE;
>  	}
>  

this all seems fine.


> @@ -1205,10 +1223,12 @@ unsigned int ata_scsiop_mode_sense(struc
>  	if (six_byte) {
>  		output_len--;
>  		rbuf[0] = output_len;
> +		rbuf[2] |= ata_id_has_fua(args->id) ? 0x10 : 0;
>  	} else {
>  		output_len -= 2;
>  		rbuf[0] = output_len >> 8;
>  		rbuf[1] = output_len;
> +		rbuf[3] |= ata_id_has_fua(args->id) ? 0x10 : 0;
>  	}

I wonder what a SCSI person thinks about this.  Its defined as 'DPO and 
FUA' not just 'FUA'.

Also, a bit of style:  please use "1 << n" for bit constants in libata.

	Jeff



  reply	other threads:[~2005-06-05  7:03 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-06-05  5:57 [PATCH Linux 2.6.12-rc5-mm2 00/09] blk: ordered request reimplementation (take 2, for review) Tejun Heo
2005-06-05  5:57 ` [PATCH Linux 2.6.12-rc5-mm2 01/09] blk: add @uptodate to end_that_request_last() and @error to rq_end_io_fn() Tejun Heo
2005-06-05  5:57 ` [PATCH Linux 2.6.12-rc5-mm2 02/09] blk: make scsi use -EOPNOTSUPP instead of -EIO on ILLEGAL_REQUEST Tejun Heo
2005-06-05  7:10   ` Jeff Garzik
2005-06-07  1:34     ` Tejun Heo
2005-06-05  5:57 ` [PATCH Linux 2.6.12-rc5-mm2 03/09] blk: make ide use -EOPNOTSUPP instead of -EIO on ABRT_ERR Tejun Heo
2005-06-05  5:57 ` [PATCH Linux 2.6.12-rc5-mm2 04/09] blk: separate out bio init part from __make_request Tejun Heo
2005-06-05  5:57 ` [PATCH Linux 2.6.12-rc5-mm2 05/09] blk: reimplement handling of barrier request Tejun Heo
2005-06-05  5:57 ` [PATCH Linux 2.6.12-rc5-mm2 06/09] blk: update SCSI to use the new blk_ordered Tejun Heo
2005-06-05  7:08   ` Jeff Garzik
2005-06-07  1:58     ` Tejun Heo
2005-06-05  5:57 ` [PATCH Linux 2.6.12-rc5-mm2 07/09] blk: update libata " Tejun Heo
2005-06-05  7:02   ` Jeff Garzik [this message]
2005-06-07  2:11     ` Tejun Heo
2005-06-05  5:57 ` [PATCH Linux 2.6.12-rc5-mm2 08/09] blk: update IDE " Tejun Heo
2005-06-05  6:47   ` Jeff Garzik
2005-06-05 14:14     ` Bartlomiej Zolnierkiewicz
2005-06-07  2:26       ` Tejun Heo
2005-06-05  5:57 ` [PATCH Linux 2.6.12-rc5-mm2 09/09] blk: debug messages Tejun Heo

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=42A2A39B.5020103@pobox.com \
    --to=jgarzik@pobox.com \
    --cc=James.Bottomley@steeleye.com \
    --cc=axboe@suse.de \
    --cc=bzolnier@gmail.com \
    --cc=htejun@gmail.com \
    --cc=linux-kernel@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