[PATCH] SCSI st: expand ability to write immediate filemarks

[PATCH] SCSI st: expand ability to write immediate filemarks

[Lee Duncan]

The st tape driver recently added the MTWEOFI ioctl, which writes
a tape filemark (EOF), like the MTWEOF ioctl, except that MTWEOFI
returns immediately. This makes certain applications, like backup
software, run much more quickly on buffered tape drives.

Since legacy applications do not know about this new MTWEOFI ioctl,
this patch adds a new ioctl option that tells the st driver to return
immediately when writing an EOF (i.e. a filemark). This new flag
is much like the existing flag that tells the st driver to perform
writes (and certain other IOs) immediately, but this new flag only
applies to writing EOFs.

This new feature is controlled via the MTSETDRVBUFFER ioctl, using
the newly-defined MT_ST_NOWAIT_EOF flag.

Use of this new feature is displayed via the sysfs tape "options"
attribute.

The st documentation was updated to mention this new flag, as well
as the problems that can occur from using it.

Signed-off-by: Lee Duncan <lduncan@suse.com>
---
 Documentation/scsi/st.txt |    4 ++++
 drivers/scsi/st.c         |   21 ++++++++++++++++++---
 drivers/scsi/st.h         |    1 +
 include/linux/mtio.h      |    1 +
 4 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/Documentation/scsi/st.txt b/Documentation/scsi/st.txt
index 691ca29..14b6e5a 100644
--- a/Documentation/scsi/st.txt
+++ b/Documentation/scsi/st.txt
@@ -390,6 +390,10 @@ MTSETDRVBUFFER
 	     MT_ST_SYSV sets the SYSV semantics (mode)
 	     MT_ST_NOWAIT enables immediate mode (i.e., don't wait for
 	        the command to finish) for some commands (e.g., rewind)
+	     MT_ST_NOWAIT_EOF enables immediate filemark mode (i.e. when
+	        writing a filemark, don't wait for it to complete). Please
+		see the BASICS note about MTWEOFI with respect to the
+		possible dangers of writing immediate filemarks.
 	     MT_ST_SILI enables setting the SILI bit in SCSI commands when
 		reading in variable block mode to enhance performance when
 		reading blocks shorter than the byte count; set this only
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 9262cdf..8c663e1 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -1106,6 +1106,12 @@ static int check_tape(struct scsi_tape *STp, struct file *filp)
                                     STp->drv_buffer));
 		}
 		STp->drv_write_prot = ((STp->buffer)->b_data[2] & 0x80) != 0;
+		if (!STp->drv_buffer && STp->immediate_filemark) {
+			printk(KERN_WARNING
+			    "%s: non-buffered tape: disabling writing immediate filemarks\n",
+			    name);
+			STp->immediate_filemark = 0;
+		}
 	}
 	st_release_request(SRpnt);
 	SRpnt = NULL;
@@ -1314,6 +1320,8 @@ static int st_flush(struct file *filp, fl_owner_t id)
 
 		memset(cmd, 0, MAX_COMMAND_SIZE);
 		cmd[0] = WRITE_FILEMARKS;
+		if (STp->immediate_filemark)
+			cmd[1] = 1;
 		cmd[4] = 1 + STp->two_fm;
 
 		SRpnt = st_do_scsi(NULL, STp, cmd, 0, DMA_NONE,
@@ -2181,8 +2189,9 @@ static void st_log_options(struct scsi_tape * STp, struct st_modedef * STm, char
 		       name, STm->defaults_for_writes, STp->omit_blklims, STp->can_partitions,
 		       STp->scsi2_logical);
 		printk(KERN_INFO
-		       "%s:    sysv: %d nowait: %d sili: %d\n", name, STm->sysv, STp->immediate,
-			STp->sili);
+		       "%s:    sysv: %d nowait: %d sili: %d nowait_filemark: %d\n",
+		       name, STm->sysv, STp->immediate, STp->sili,
+		       STp->immediate_filemark);
 		printk(KERN_INFO "%s:    debugging: %d\n",
 		       name, debugging);
 	}
@@ -2224,6 +2233,7 @@ static int st_set_options(struct scsi_tape *STp, long options)
 			STp->can_partitions = (options & MT_ST_CAN_PARTITIONS) != 0;
 		STp->scsi2_logical = (options & MT_ST_SCSI2LOGICAL) != 0;
 		STp->immediate = (options & MT_ST_NOWAIT) != 0;
+		STp->immediate_filemark = (options & MT_ST_NOWAIT_EOF) != 0;
 		STm->sysv = (options & MT_ST_SYSV) != 0;
 		STp->sili = (options & MT_ST_SILI) != 0;
 		DEB( debugging = (options & MT_ST_DEBUGGING) != 0;
@@ -2255,6 +2265,8 @@ static int st_set_options(struct scsi_tape *STp, long options)
 			STp->scsi2_logical = value;
 		if ((options & MT_ST_NOWAIT) != 0)
 			STp->immediate = value;
+		if ((options & MT_ST_NOWAIT_EOF) != 0)
+			STp->immediate_filemark = value;
 		if ((options & MT_ST_SYSV) != 0)
 			STm->sysv = value;
 		if ((options & MT_ST_SILI) != 0)
@@ -2714,7 +2726,8 @@ static int st_int_ioctl(struct scsi_tape *STp, unsigned int cmd_in, unsigned lon
 		cmd[0] = WRITE_FILEMARKS;
 		if (cmd_in == MTWSM)
 			cmd[1] = 2;
-		if (cmd_in == MTWEOFI)
+		if (cmd_in == MTWEOFI ||
+		    (cmd_in == MTWEOF && STp->immediate_filemark))
 			cmd[1] |= 1;
 		cmd[2] = (arg >> 16);
 		cmd[3] = (arg >> 8);
@@ -4093,6 +4106,7 @@ static int st_probe(struct device *dev)
 	tpnt->scsi2_logical = ST_SCSI2LOGICAL;
 	tpnt->sili = ST_SILI;
 	tpnt->immediate = ST_NOWAIT;
+	tpnt->immediate_filemark = 0;
 	tpnt->default_drvbuffer = 0xff;		/* No forced buffering */
 	tpnt->partition = 0;
 	tpnt->new_partition = 0;
@@ -4478,6 +4492,7 @@ st_options_show(struct device *dev, struct device_attribute *attr, char *buf)
 	options |= STp->scsi2_logical ? MT_ST_SCSI2LOGICAL : 0;
 	options |= STm->sysv ? MT_ST_SYSV : 0;
 	options |= STp->immediate ? MT_ST_NOWAIT : 0;
+	options |= STp->immediate_filemark ? MT_ST_NOWAIT_EOF : 0;
 	options |= STp->sili ? MT_ST_SILI : 0;
 
 	l = snprintf(buf, PAGE_SIZE, "0x%08x\n", options);
diff --git a/drivers/scsi/st.h b/drivers/scsi/st.h
index f91a67c..ea35632 100644
--- a/drivers/scsi/st.h
+++ b/drivers/scsi/st.h
@@ -120,6 +120,7 @@ struct scsi_tape {
 	unsigned char c_algo;			/* compression algorithm */
 	unsigned char pos_unknown;			/* after reset position unknown */
 	unsigned char sili;			/* use SILI when reading in variable b mode */
+	unsigned char immediate_filemark;	/* write filemark immediately */
 	int tape_type;
 	int long_timeout;	/* timeout for commands known to take long time */
 
diff --git a/include/linux/mtio.h b/include/linux/mtio.h
index 8f82575..18543e2 100644
--- a/include/linux/mtio.h
+++ b/include/linux/mtio.h
@@ -194,6 +194,7 @@ struct	mtpos {
 #define MT_ST_SYSV              0x1000
 #define MT_ST_NOWAIT            0x2000
 #define MT_ST_SILI		0x4000
+#define MT_ST_NOWAIT_EOF	0x8000
 
 /* The mode parameters to be controlled. Parameter chosen with bits 20-28 */
 #define MT_ST_CLEAR_DEFAULT	0xfffff
-- 
1.7.8.3

Re: [PATCH] SCSI st: expand ability to write immediate filemarks

[Kai Makisara]

On Thu, 1 Mar 2012, Lee Duncan wrote:

> The st tape driver recently added the MTWEOFI ioctl, which writes
> a tape filemark (EOF), like the MTWEOF ioctl, except that MTWEOFI
> returns immediately. This makes certain applications, like backup
> software, run much more quickly on buffered tape drives.
> 
> Since legacy applications do not know about this new MTWEOFI ioctl,
> this patch adds a new ioctl option that tells the st driver to return
> immediately when writing an EOF (i.e. a filemark). This new flag
> is much like the existing flag that tells the st driver to perform
> writes (and certain other IOs) immediately, but this new flag only
> applies to writing EOFs.
> 
> This new feature is controlled via the MTSETDRVBUFFER ioctl, using
> the newly-defined MT_ST_NOWAIT_EOF flag.
> 
> Use of this new feature is displayed via the sysfs tape "options"
> attribute.
> 
> The st documentation was updated to mention this new flag, as well
> as the problems that can occur from using it.
> 
> Signed-off-by: Lee Duncan <lduncan@suse.com>
Acked-by: Kai Makisara <kai.makisara@kolumbus.fi>

> ---
>  Documentation/scsi/st.txt |    4 ++++
>  drivers/scsi/st.c         |   21 ++++++++++++++++++---
>  drivers/scsi/st.h         |    1 +
>  include/linux/mtio.h      |    1 +
>  4 files changed, 24 insertions(+), 3 deletions(-)

Thanks,
Kai

[PATCH] [SCSI] st: expand ability to write immediate filemarks

[Lee Duncan]

The st tape driver recently added the MTWEOFI ioctl, which writes
a tape filemark (EOF), like the MTWEOF ioctl, except that MTWEOFI
returns immediately. This makes certain applications, like backup
software, run much more quickly on buffered tape drives.

Since legacy applications do not know about this new MTWEOFI ioctl,
this patch adds a new flag that tells the st driver to return
immediately when writing an EOF (i.e. a filemark). This new flag
is much like the existing flag that tells the st driver to perform
writes (and certain other IOs) immediately, but this new flag only
applies to writing EOFs.

This new feature can be enabled via a new module flag: st_nowait_eof.
It can also be set or cleared via the MTSETDRVBUFFER ioctl, using
the newly-defined MT_ST_NOWAIT_EOF flag.

Lastly, this new feature is displayed via the sysfs tape "options"
attribute.

Signed-off-by: Lee Duncan <lduncan@suse.com>
---
 Documentation/scsi/st.txt |    2 ++
 drivers/scsi/st.c         |   24 +++++++++++++++++++++---
 drivers/scsi/st.h         |    1 +
 include/linux/mtio.h      |    1 +
 4 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/Documentation/scsi/st.txt b/Documentation/scsi/st.txt
index 691ca29..85c1b1a 100644
--- a/Documentation/scsi/st.txt
+++ b/Documentation/scsi/st.txt
@@ -390,6 +390,8 @@ MTSETDRVBUFFER
 	     MT_ST_SYSV sets the SYSV semantics (mode)
 	     MT_ST_NOWAIT enables immediate mode (i.e., don't wait for
 	        the command to finish) for some commands (e.g., rewind)
+	     MT_ST_NOWAIT_EOF enables immediate filemark mode (i.e. when
+	        writing a filemark, don't wait for it to complete)
 	     MT_ST_SILI enables setting the SILI bit in SCSI commands when
 		reading in variable block mode to enhance performance when
 		reading blocks shorter than the byte count; set this only
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 9b28f39..f18d10b 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -81,6 +81,7 @@ static int max_sg_segs;
 static int try_direct_io = TRY_DIRECT_IO;
 static int try_rdio = 1;
 static int try_wdio = 1;
+static int st_nowait_eof;
 
 static int st_dev_max;
 static int st_nr_dev;
@@ -103,6 +104,8 @@ module_param_named(max_sg_segs, max_sg_segs, int, 0);
 MODULE_PARM_DESC(max_sg_segs, "Maximum number of scatter/gather segments to use (256)");
 module_param_named(try_direct_io, try_direct_io, int, 0);
 MODULE_PARM_DESC(try_direct_io, "Try direct I/O between user buffer and tape drive (1)");
+module_param_named(st_nowait_eof, st_nowait_eof, int, 0);
+MODULE_PARM_DESC(st_nowait_eof, "Do not wait when writing EOF (filemark) (0)");
 
 /* Extra parameters for testing */
 module_param_named(try_rdio, try_rdio, int, 0);
@@ -1106,6 +1109,12 @@ static int check_tape(struct scsi_tape *STp, struct file *filp)
                                     STp->drv_buffer));
 		}
 		STp->drv_write_prot = ((STp->buffer)->b_data[2] & 0x80) != 0;
+		if (!STp->drv_buffer && STp->immediate_filemark) {
+			printk(KERN_WARNING
+			    "%s: non-buffered tape: disabling writing immediate filemarks\n",
+			    name);
+			STp->immediate_filemark = 0;
+		}
 	}
 	st_release_request(SRpnt);
 	SRpnt = NULL;
@@ -1306,6 +1315,8 @@ static int st_flush(struct file *filp, fl_owner_t id)
 
 		memset(cmd, 0, MAX_COMMAND_SIZE);
 		cmd[0] = WRITE_FILEMARKS;
+		if (STp->immediate_filemark)
+			cmd[1] = 1;
 		cmd[4] = 1 + STp->two_fm;
 
 		SRpnt = st_do_scsi(NULL, STp, cmd, 0, DMA_NONE,
@@ -2172,8 +2183,9 @@ static void st_log_options(struct scsi_tape * STp, struct st_modedef * STm, char
 		       name, STm->defaults_for_writes, STp->omit_blklims, STp->can_partitions,
 		       STp->scsi2_logical);
 		printk(KERN_INFO
-		       "%s:    sysv: %d nowait: %d sili: %d\n", name, STm->sysv, STp->immediate,
-			STp->sili);
+		       "%s:    sysv: %d nowait: %d sili: %d nowait_filemark: %d\n",
+		       name, STm->sysv, STp->immediate, STp->sili,
+		       STp->immediate_filemark);
 		printk(KERN_INFO "%s:    debugging: %d\n",
 		       name, debugging);
 	}
@@ -2215,6 +2227,7 @@ static int st_set_options(struct scsi_tape *STp, long options)
 			STp->can_partitions = (options & MT_ST_CAN_PARTITIONS) != 0;
 		STp->scsi2_logical = (options & MT_ST_SCSI2LOGICAL) != 0;
 		STp->immediate = (options & MT_ST_NOWAIT) != 0;
+		STp->immediate_filemark = (options & MT_ST_NOWAIT_EOF) != 0;
 		STm->sysv = (options & MT_ST_SYSV) != 0;
 		STp->sili = (options & MT_ST_SILI) != 0;
 		DEB( debugging = (options & MT_ST_DEBUGGING) != 0;
@@ -2246,6 +2259,8 @@ static int st_set_options(struct scsi_tape *STp, long options)
 			STp->scsi2_logical = value;
 		if ((options & MT_ST_NOWAIT) != 0)
 			STp->immediate = value;
+		if ((options & MT_ST_NOWAIT_EOF) != 0)
+			STp->immediate_filemark = value;
 		if ((options & MT_ST_SYSV) != 0)
 			STm->sysv = value;
 		if ((options & MT_ST_SILI) != 0)
@@ -2705,7 +2720,8 @@ static int st_int_ioctl(struct scsi_tape *STp, unsigned int cmd_in, unsigned lon
 		cmd[0] = WRITE_FILEMARKS;
 		if (cmd_in == MTWSM)
 			cmd[1] = 2;
-		if (cmd_in == MTWEOFI)
+		if (cmd_in == MTWEOFI ||
+		    (cmd_in == MTWEOF && STp->immediate_filemark))
 			cmd[1] |= 1;
 		cmd[2] = (arg >> 16);
 		cmd[3] = (arg >> 8);
@@ -4084,6 +4100,7 @@ static int st_probe(struct device *dev)
 	tpnt->scsi2_logical = ST_SCSI2LOGICAL;
 	tpnt->sili = ST_SILI;
 	tpnt->immediate = ST_NOWAIT;
+	tpnt->immediate_filemark = st_nowait_eof;
 	tpnt->default_drvbuffer = 0xff;		/* No forced buffering */
 	tpnt->partition = 0;
 	tpnt->new_partition = 0;
@@ -4467,6 +4484,7 @@ st_options_show(struct device *dev, struct device_attribute *attr, char *buf)
 	options |= STp->scsi2_logical ? MT_ST_SCSI2LOGICAL : 0;
 	options |= STm->sysv ? MT_ST_SYSV : 0;
 	options |= STp->immediate ? MT_ST_NOWAIT : 0;
+	options |= STp->immediate_filemark ? MT_ST_NOWAIT_EOF : 0;
 	options |= STp->sili ? MT_ST_SILI : 0;
 
 	l = snprintf(buf, PAGE_SIZE, "0x%08x\n", options);
diff --git a/drivers/scsi/st.h b/drivers/scsi/st.h
index f91a67c..ea35632 100644
--- a/drivers/scsi/st.h
+++ b/drivers/scsi/st.h
@@ -120,6 +120,7 @@ struct scsi_tape {
 	unsigned char c_algo;			/* compression algorithm */
 	unsigned char pos_unknown;			/* after reset position unknown */
 	unsigned char sili;			/* use SILI when reading in variable b mode */
+	unsigned char immediate_filemark;	/* write filemark immediately */
 	int tape_type;
 	int long_timeout;	/* timeout for commands known to take long time */
 
diff --git a/include/linux/mtio.h b/include/linux/mtio.h
index 8f82575..18543e2 100644
--- a/include/linux/mtio.h
+++ b/include/linux/mtio.h
@@ -194,6 +194,7 @@ struct	mtpos {
 #define MT_ST_SYSV              0x1000
 #define MT_ST_NOWAIT            0x2000
 #define MT_ST_SILI		0x4000
+#define MT_ST_NOWAIT_EOF	0x8000
 
 /* The mode parameters to be controlled. Parameter chosen with bits 20-28 */
 #define MT_ST_CLEAR_DEFAULT	0xfffff
-- 
1.7.8.3

Re: [PATCH] [SCSI] st: expand ability to write immediate filemarks

[Kai Makisara]

On Wed, 15 Feb 2012, Lee Duncan wrote:

> The st tape driver recently added the MTWEOFI ioctl, which writes
> a tape filemark (EOF), like the MTWEOF ioctl, except that MTWEOFI
> returns immediately. This makes certain applications, like backup
> software, run much more quickly on buffered tape drives.
> 
> Since legacy applications do not know about this new MTWEOFI ioctl,
> this patch adds a new flag that tells the st driver to return
> immediately when writing an EOF (i.e. a filemark). This new flag
> is much like the existing flag that tells the st driver to perform
> writes (and certain other IOs) immediately, but this new flag only
> applies to writing EOFs.
> 
> This new feature can be enabled via a new module flag: st_nowait_eof.
> It can also be set or cleared via the MTSETDRVBUFFER ioctl, using
> the newly-defined MT_ST_NOWAIT_EOF flag.
> 
> Lastly, this new feature is displayed via the sysfs tape "options"
> attribute.
> 
> Signed-off-by: Lee Duncan <lduncan@suse.com>
> ---
>  Documentation/scsi/st.txt |    2 ++
>  drivers/scsi/st.c         |   24 +++++++++++++++++++++---
>  drivers/scsi/st.h         |    1 +
>  include/linux/mtio.h      |    1 +
>  4 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/scsi/st.txt b/Documentation/scsi/st.txt
> index 691ca29..85c1b1a 100644
> --- a/Documentation/scsi/st.txt
> +++ b/Documentation/scsi/st.txt
> @@ -390,6 +390,8 @@ MTSETDRVBUFFER
>  	     MT_ST_SYSV sets the SYSV semantics (mode)
>  	     MT_ST_NOWAIT enables immediate mode (i.e., don't wait for
>  	        the command to finish) for some commands (e.g., rewind)
> +	     MT_ST_NOWAIT_EOF enables immediate filemark mode (i.e. when
> +	        writing a filemark, don't wait for it to complete)

You could add a note that errors before EOF may not be noticed.

>  	     MT_ST_SILI enables setting the SILI bit in SCSI commands when
>  		reading in variable block mode to enhance performance when
>  		reading blocks shorter than the byte count; set this only
> diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
> index 9b28f39..f18d10b 100644
> --- a/drivers/scsi/st.c
> +++ b/drivers/scsi/st.c
> @@ -81,6 +81,7 @@ static int max_sg_segs;
>  static int try_direct_io = TRY_DIRECT_IO;
>  static int try_rdio = 1;
>  static int try_wdio = 1;
> +static int st_nowait_eof;
>  
>  static int st_dev_max;
>  static int st_nr_dev;
> @@ -103,6 +104,8 @@ module_param_named(max_sg_segs, max_sg_segs, int, 0);
>  MODULE_PARM_DESC(max_sg_segs, "Maximum number of scatter/gather segments to use (256)");
>  module_param_named(try_direct_io, try_direct_io, int, 0);
>  MODULE_PARM_DESC(try_direct_io, "Try direct I/O between user buffer and tape drive (1)");
> +module_param_named(st_nowait_eof, st_nowait_eof, int, 0);
> +MODULE_PARM_DESC(st_nowait_eof, "Do not wait when writing EOF (filemark) (0)");
>  

I think this should not be a module option. The property should be 
settable only as a mode option like other options of this kind.
(I may have not written this clearly in my previous reply).

Otherwise the patch looks good.

Kai

Re: [PATCH] [SCSI] st: expand ability to write immediate filemarks

[Lee Duncan]

Hi Kai:

Thanks for your response ...

On 02/28/2012 11:40 AM, Kai Makisara wrote:
> On Wed, 15 Feb 2012, Lee Duncan wrote:
> 
>> The st tape driver recently added the MTWEOFI ioctl, which writes
>> a tape filemark (EOF), like the MTWEOF ioctl, except that MTWEOFI
>> returns immediately. This makes certain applications, like backup
>> software, run much more quickly on buffered tape drives.
>>
>> Since legacy applications do not know about this new MTWEOFI ioctl,
>> this patch adds a new flag that tells the st driver to return
>> immediately when writing an EOF (i.e. a filemark). This new flag
>> is much like the existing flag that tells the st driver to perform
>> writes (and certain other IOs) immediately, but this new flag only
>> applies to writing EOFs.
>>
>> This new feature can be enabled via a new module flag: st_nowait_eof.
>> It can also be set or cleared via the MTSETDRVBUFFER ioctl, using
>> the newly-defined MT_ST_NOWAIT_EOF flag.
>>
>> Lastly, this new feature is displayed via the sysfs tape "options"
>> attribute.
>>
>> Signed-off-by: Lee Duncan <lduncan@suse.com>
>> ---
>>  Documentation/scsi/st.txt |    2 ++
>>  drivers/scsi/st.c         |   24 +++++++++++++++++++++---
>>  drivers/scsi/st.h         |    1 +
>>  include/linux/mtio.h      |    1 +
>>  4 files changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/scsi/st.txt b/Documentation/scsi/st.txt
>> index 691ca29..85c1b1a 100644
>> --- a/Documentation/scsi/st.txt
>> +++ b/Documentation/scsi/st.txt
>> @@ -390,6 +390,8 @@ MTSETDRVBUFFER
>>  	     MT_ST_SYSV sets the SYSV semantics (mode)
>>  	     MT_ST_NOWAIT enables immediate mode (i.e., don't wait for
>>  	        the command to finish) for some commands (e.g., rewind)
>> +	     MT_ST_NOWAIT_EOF enables immediate filemark mode (i.e. when
>> +	        writing a filemark, don't wait for it to complete)
> 
> You could add a note that errors before EOF may not be noticed.

I will be glad to add such a note.

> 
>>  	     MT_ST_SILI enables setting the SILI bit in SCSI commands when
>>  		reading in variable block mode to enhance performance when
>>  		reading blocks shorter than the byte count; set this only
>> diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
>> index 9b28f39..f18d10b 100644
>> --- a/drivers/scsi/st.c
>> +++ b/drivers/scsi/st.c
>> @@ -81,6 +81,7 @@ static int max_sg_segs;
>>  static int try_direct_io = TRY_DIRECT_IO;
>>  static int try_rdio = 1;
>>  static int try_wdio = 1;
>> +static int st_nowait_eof;
>>  
>>  static int st_dev_max;
>>  static int st_nr_dev;
>> @@ -103,6 +104,8 @@ module_param_named(max_sg_segs, max_sg_segs, int, 0);
>>  MODULE_PARM_DESC(max_sg_segs, "Maximum number of scatter/gather segments to use (256)");
>>  module_param_named(try_direct_io, try_direct_io, int, 0);
>>  MODULE_PARM_DESC(try_direct_io, "Try direct I/O between user buffer and tape drive (1)");
>> +module_param_named(st_nowait_eof, st_nowait_eof, int, 0);
>> +MODULE_PARM_DESC(st_nowait_eof, "Do not wait when writing EOF (filemark) (0)");
>>  
> 
> I think this should not be a module option. The property should be 
> settable only as a mode option like other options of this kind.
> (I may have not written this clearly in my previous reply).
> 

I respectfully disagree.

There are legacy applications, such as the IBM network backup program that sparked this bug fix. Such applications do not know about the capability added by this patch, just as they do not know about the relatively new MTWEOFI ioctl. Hence the module option is a convenient method for such applications to achieve increased throughput.

I agree there is a small risk of loosing errors or error context when writing immediate filemarks, but is the same problem that already exists in the case of the MTWEOFI ioctl.

I would be glad to add a section in the st documentation that mentions the dangers of writing immediate filemarks, if you think that would help clarify usage of this feature.


> Otherwise the patch looks good.
> 

-- 
Lee Duncan
SUSE Labs

Re: [PATCH] [SCSI] st: expand ability to write immediate filemarks

[Kai Mäkisara]

On 02/29/2012 12:03 AM, Lee Duncan wrote:
> Hi Kai:
>
> Thanks for your response ...
>
> On 02/28/2012 11:40 AM, Kai Makisara wrote:
>> On Wed, 15 Feb 2012, Lee Duncan wrote:
>>
>>> The st tape driver recently added the MTWEOFI ioctl, which writes
>>> a tape filemark (EOF), like the MTWEOF ioctl, except that MTWEOFI
>>> returns immediately. This makes certain applications, like backup
>>> software, run much more quickly on buffered tape drives.
>>>
...
>>>
>>>   static int st_dev_max;
>>>   static int st_nr_dev;
>>> @@ -103,6 +104,8 @@ module_param_named(max_sg_segs, max_sg_segs, int, 0);
>>>   MODULE_PARM_DESC(max_sg_segs, "Maximum number of scatter/gather segments to use (256)");
>>>   module_param_named(try_direct_io, try_direct_io, int, 0);
>>>   MODULE_PARM_DESC(try_direct_io, "Try direct I/O between user buffer and tape drive (1)");
>>> +module_param_named(st_nowait_eof, st_nowait_eof, int, 0);
>>> +MODULE_PARM_DESC(st_nowait_eof, "Do not wait when writing EOF (filemark) (0)");
>>>
>>
>> I think this should not be a module option. The property should be
>> settable only as a mode option like other options of this kind.
>> (I may have not written this clearly in my previous reply).
>>
>
> I respectfully disagree.
>
> There are legacy applications, such as the IBM network backup program that sparked this
> bug fix. Such applications do not know about the capability added by this patch, just as
> they do not know about the relatively new MTWEOFI ioctl. Hence the module option is a
> convenient method for such applications to achieve increased throughput.
>
You may not have fully understood what there mode options mean in 
practice. For each physical tape, you have up to four different device
files (modes; eight if you count both auto-rewind and non-rewind 
devices). Each can be programmed with different characteristics using 
the ioctls. For instance, you can have /dev/nst0a which uses "normal" 
WEOF and /dev/nst0b which uses immediate WEOF. If you want the program 
to benefit from immeadiate WEOFs, you tell it to use /dev/nst0b. The 
program does not have to know anything about the new options.

The system should configure the devices when detected, according to the 
choices made by the system manager. I once made a proof-of-concept 
program stinit for this purpose. mt can also be used for this. The trend 
is systems has for tens of years to move from boot-time configuration to 
run-time configuration.

> I agree there is a small risk of loosing errors or error context when writing immediate
> filemarks, but is the same problem that already exists in the case of the MTWEOFI ioctl.
>
Yes, but in case of MWEOFI, the programmer takes a calculated risk. For 
instance, he/she can use MTWEOF for the last filemark to catch the 
errors (or just look at possible errors from close()). The defaults are 
safe. In case of a module option, the default for the unsuspecting user 
is unsafe.

> I would be glad to add a section in the st documentation that mentions the dangers of
> writing immediate filemarks, if you think that would help clarify usage of this feature.
>
It helps clarity but it does not remove the danger.

Kai

Re: [PATCH] [SCSI] st: expand ability to write immediate filemarks

[Lee Duncan]

Kai:

Your argument makes sense. I will resubmit the patch without the module
variable, and with a little more added to the documentation concerning
the possible dangers of writing immediate EOFs.

On 02/28/2012 11:05 PM, Kai Mäkisara wrote:
> On 02/29/2012 12:03 AM, Lee Duncan wrote:
>> Hi Kai:
>>
>> Thanks for your response ...
>>
>> On 02/28/2012 11:40 AM, Kai Makisara wrote:
>>> On Wed, 15 Feb 2012, Lee Duncan wrote:
>>>
>>>> The st tape driver recently added the MTWEOFI ioctl, which writes
>>>> a tape filemark (EOF), like the MTWEOF ioctl, except that MTWEOFI
>>>> returns immediately. This makes certain applications, like backup
>>>> software, run much more quickly on buffered tape drives.
>>>>
> ...
>>>>
>>>>   static int st_dev_max;
>>>>   static int st_nr_dev;
>>>> @@ -103,6 +104,8 @@ module_param_named(max_sg_segs, max_sg_segs,
>>>> int, 0);
>>>>   MODULE_PARM_DESC(max_sg_segs, "Maximum number of scatter/gather
>>>> segments to use (256)");
>>>>   module_param_named(try_direct_io, try_direct_io, int, 0);
>>>>   MODULE_PARM_DESC(try_direct_io, "Try direct I/O between user
>>>> buffer and tape drive (1)");
>>>> +module_param_named(st_nowait_eof, st_nowait_eof, int, 0);
>>>> +MODULE_PARM_DESC(st_nowait_eof, "Do not wait when writing EOF
>>>> (filemark) (0)");
>>>>
>>>
>>> I think this should not be a module option. The property should be
>>> settable only as a mode option like other options of this kind.
>>> (I may have not written this clearly in my previous reply).
>>>
>>
>> I respectfully disagree.
>>
>> There are legacy applications, such as the IBM network backup program
>> that sparked this
>> bug fix. Such applications do not know about the capability added by
>> this patch, just as
>> they do not know about the relatively new MTWEOFI ioctl. Hence the
>> module option is a
>> convenient method for such applications to achieve increased throughput.
>>
> You may not have fully understood what there mode options mean in
> practice. For each physical tape, you have up to four different device
> files (modes; eight if you count both auto-rewind and non-rewind
> devices). Each can be programmed with different characteristics using
> the ioctls. For instance, you can have /dev/nst0a which uses "normal"
> WEOF and /dev/nst0b which uses immediate WEOF. If you want the program
> to benefit from immeadiate WEOFs, you tell it to use /dev/nst0b. The
> program does not have to know anything about the new options.
> 
> The system should configure the devices when detected, according to the
> choices made by the system manager. I once made a proof-of-concept
> program stinit for this purpose. mt can also be used for this. The trend
> is systems has for tens of years to move from boot-time configuration to
> run-time configuration.
> 
>> I agree there is a small risk of loosing errors or error context when
>> writing immediate
>> filemarks, but is the same problem that already exists in the case of
>> the MTWEOFI ioctl.
>>
> Yes, but in case of MWEOFI, the programmer takes a calculated risk. For
> instance, he/she can use MTWEOF for the last filemark to catch the
> errors (or just look at possible errors from close()). The defaults are
> safe. In case of a module option, the default for the unsuspecting user
> is unsafe.
> 
>> I would be glad to add a section in the st documentation that mentions
>> the dangers of
>> writing immediate filemarks, if you think that would help clarify
>> usage of this feature.
>>
> It helps clarity but it does not remove the danger.
> 
> Kai
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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