Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754861AbaAVKMc (ORCPT ); Wed, 22 Jan 2014 05:12:32 -0500 Received: from mail-wg0-f49.google.com ([74.125.82.49]:32852 "EHLO mail-wg0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753419AbaAVKMa (ORCPT ); Wed, 22 Jan 2014 05:12:30 -0500 Message-ID: <52DF9989.5050008@dev.mellanox.co.il> Date: Wed, 22 Jan 2014 12:12:25 +0200 From: Sagi Grimberg User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: "Nicholas A. Bellinger" CC: "Nicholas A. Bellinger" , target-devel , linux-scsi , linux-kernel , "Martin K. Petersen" , Christoph Hellwig , Hannes Reinecke , Sagi Grimberg , Or Gerlitz , Roland Dreier Subject: Re: [PATCH-v2 12/17] target/file: Add DIF protection init/format support References: <1390099480-29013-1-git-send-email-nab@daterainc.com> <1390099480-29013-13-git-send-email-nab@daterainc.com> <52DBC592.6010405@dev.mellanox.co.il> <1390343288.5567.782.camel@haakon3.risingtidesystems.com> In-Reply-To: <1390343288.5567.782.camel@haakon3.risingtidesystems.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 1/22/2014 12:28 AM, Nicholas A. Bellinger wrote: > On Sun, 2014-01-19 at 14:31 +0200, Sagi Grimberg wrote: >> On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote: >>> From: Nicholas Bellinger >>> >>> This patch adds support for DIF protection init/format support into >>> the FILEIO backend. >>> >>> It involves using a seperate $FILE.protection for storing PI that is >>> opened via fd_init_prot() using the common pi_prot_type attribute. >>> The actual formatting of the protection is done via fd_format_prot() >>> using the common pi_prot_format attribute, that will populate the >>> initial PI data based upon the currently configured pi_prot_type. >>> >>> Based on original FILEIO code from Sagi. >> Nice! see comments below... >> >>> v1 changes: >>> - Fix sparse warnings in fd_init_format_buf (Fengguang) >>> >>> Cc: Martin K. Petersen >>> Cc: Christoph Hellwig >>> Cc: Hannes Reinecke >>> Cc: Sagi Grimberg >>> Cc: Or Gerlitz >>> Signed-off-by: Nicholas Bellinger >>> --- >>> drivers/target/target_core_file.c | 137 +++++++++++++++++++++++++++++++++++++ >>> drivers/target/target_core_file.h | 4 ++ >>> 2 files changed, 141 insertions(+) >>> >>> diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c >>> index 0e34cda..119d519 100644 >>> --- a/drivers/target/target_core_file.c >>> +++ b/drivers/target/target_core_file.c >>> @@ -700,6 +700,140 @@ static sector_t fd_get_blocks(struct se_device *dev) >>> dev->dev_attrib.block_size); >>> } >>> >>> +static int fd_init_prot(struct se_device *dev) >>> +{ >>> + struct fd_dev *fd_dev = FD_DEV(dev); >>> + struct file *prot_file, *file = fd_dev->fd_file; >>> + struct inode *inode; >>> + int ret, flags = O_RDWR | O_CREAT | O_LARGEFILE | O_DSYNC; >>> + char buf[FD_MAX_DEV_PROT_NAME]; >>> + >>> + if (!file) { >>> + pr_err("Unable to locate fd_dev->fd_file\n"); >>> + return -ENODEV; >>> + } >>> + >>> + inode = file->f_mapping->host; >>> + if (S_ISBLK(inode->i_mode)) { >>> + pr_err("FILEIO Protection emulation only supported on" >>> + " !S_ISBLK\n"); >>> + return -ENOSYS; >>> + } >>> + >>> + if (fd_dev->fbd_flags & FDBD_HAS_BUFFERED_IO_WCE) >>> + flags &= ~O_DSYNC; >>> + >>> + snprintf(buf, FD_MAX_DEV_PROT_NAME, "%s.protection", >>> + fd_dev->fd_dev_name); >>> + >>> + prot_file = filp_open(buf, flags, 0600); >>> + if (IS_ERR(prot_file)) { >>> + pr_err("filp_open(%s) failed\n", buf); >>> + ret = PTR_ERR(prot_file); >>> + return ret; >>> + } >>> + fd_dev->fd_prot_file = prot_file; >>> + >>> + return 0; >>> +} >>> + >>> +static void fd_init_format_buf(struct se_device *dev, unsigned char *buf, >>> + u32 unit_size, u32 *ref_tag, u16 app_tag, >>> + bool inc_reftag) >>> +{ >>> + unsigned char *p = buf; >>> + int i; >>> + >>> + for (i = 0; i < unit_size; i += dev->prot_length) { >>> + *((u16 *)&p[0]) = 0xffff; >>> + *((__be16 *)&p[2]) = cpu_to_be16(app_tag); >>> + *((__be32 *)&p[4]) = cpu_to_be32(*ref_tag); >>> + >>> + if (inc_reftag) >>> + (*ref_tag)++; >>> + >>> + p += dev->prot_length; >>> + } >>> +} >>> + >>> +static int fd_format_prot(struct se_device *dev) >>> +{ >>> + struct fd_dev *fd_dev = FD_DEV(dev); >>> + struct file *prot_fd = fd_dev->fd_prot_file; >>> + sector_t prot_length, prot; >>> + unsigned char *buf; >>> + loff_t pos = 0; >>> + u32 ref_tag = 0; >>> + int unit_size = FDBD_FORMAT_UNIT_SIZE * dev->dev_attrib.block_size; >>> + int rc, ret = 0, size, len; >>> + bool inc_reftag = false; >>> + >>> + if (!dev->dev_attrib.pi_prot_type) { >>> + pr_err("Unable to format_prot while pi_prot_type == 0\n"); >>> + return -ENODEV; >>> + } >>> + if (!prot_fd) { >>> + pr_err("Unable to locate fd_dev->fd_prot_file\n"); >>> + return -ENODEV; >>> + } >>> + >>> + switch (dev->dev_attrib.pi_prot_type) { >> redundant - see below. >>> + case TARGET_DIF_TYPE3_PROT: >>> + ref_tag = 0xffffffff; >>> + break; >>> + case TARGET_DIF_TYPE2_PROT: >>> + case TARGET_DIF_TYPE1_PROT: >>> + inc_reftag = true; >>> + break; >>> + default: >>> + break; >>> + } >>> + >>> + buf = vzalloc(unit_size); >>> + if (!buf) { >>> + pr_err("Unable to allocate FILEIO prot buf\n"); >>> + return -ENOMEM; >>> + } >>> + >>> + prot_length = (dev->transport->get_blocks(dev) + 1) * dev->prot_length; >>> + size = prot_length; >>> + >>> + pr_debug("Using FILEIO prot_length: %llu\n", >>> + (unsigned long long)prot_length); >>> + >>> + for (prot = 0; prot < prot_length; prot += unit_size) { >>> + >>> + fd_init_format_buf(dev, buf, unit_size, &ref_tag, 0xffff, >>> + inc_reftag); >> I didn't send you my latest patches (my fault...).T10-PI format should >> only place >> escape values throughout the protection file (fill it with 0xff). so I >> guess in this case >> fd_init_formast_buf() boils down to memset(buf, 0xff, unit_size) once >> before the loop >> and just loop until prot_length writing buf, no need to address >> apptag/reftag... > Yeah, was thinking about just formatting with escape values as mentioned > above, but thought it might be useful to keep around for pre-populating > values apptag + reftag values for testing purposes. > > --nab > OK, but maybe it is better to do that under some debug configuration rather then always do that. Sagi. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/