[PATCH] Fix WCE and RCD bits of the caching mode page (repost)

[Arne Redlich-2]

---
Tomo,

I previously posted this patch as RFC [1]. The feedback so far was
positive [2] (thank you, Ross), so I'd like to request its
inclusion unless you see any issues with it.

Thanks,
Arne

[1] http://article.gmane.org/gmane.linux.iscsi.iscsi-target.devel/8545
[2] http://article.gmane.org/gmane.linux.iscsi.iscsi-target.devel/8548

 kernel/block-io.c    |    6 +++++-
 kernel/file-io.c     |    2 ++
 kernel/iscsi.h       |   12 +++++++++---
 kernel/target_disk.c |   15 ++++++++++-----
 kernel/volume.c      |    5 +++--
 5 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/kernel/block-io.c b/kernel/block-io.c
index 492cc20..b4b169a 100644
--- a/kernel/block-io.c
+++ b/kernel/block-io.c
@@ -318,7 +318,7 @@ blockio_detach(struct iet_volume *volume)
 }
 
 static int
-blockio_attach (struct iet_volume *volume, char *args)
+blockio_attach(struct iet_volume *volume, char *args)
 {
  struct blockio_data *bio_data;
  int err = 0;
@@ -344,6 +344,10 @@ blockio_attach (struct iet_volume *volume, char *args)
  /* Assign a vendor id, generate scsi id if none exists */
  gen_scsiid(volume, bio_data->bdev->bd_inode);
 
+ /* Offer neither write nor read caching */
+ ClearLURCache(volume);
+ ClearLUWCache(volume);
+
  volume->blk_shift = SECTOR_SIZE_BITS;
  volume->blk_cnt = bio_data->bdev->bd_inode->i_size >> volume->blk_shift;
 
diff --git a/kernel/file-io.c b/kernel/file-io.c
index b819647..db52f56 100644
--- a/kernel/file-io.c
+++ b/kernel/file-io.c
@@ -284,6 +284,8 @@ static int fileio_attach(struct iet_volume *lu, char *args)
  lu->blk_shift = SECTOR_SIZE_BITS;
  lu->blk_cnt = inode->i_size >> lu->blk_shift;
 
+ /* we're using the page cache */
+ SetLURCache(lu);
 out:
  if (err < 0)
  fileio_detach(lu);
diff --git a/kernel/iscsi.h b/kernel/iscsi.h
index 5248024..9b08534 100644
--- a/kernel/iscsi.h
+++ b/kernel/iscsi.h
@@ -153,14 +153,20 @@ struct iet_volume {
 
 enum lu_flags {
  LU_READONLY,
- LU_ASYNC,
+ LU_WCACHE,
+ LU_RCACHE,
 };
 
 #define LUReadonly(lu) test_bit(LU_READONLY, &(lu)->flags)
 #define SetLUReadonly(lu) set_bit(LU_READONLY, &(lu)->flags)
 
-#define LUAsync(lu) test_bit(LU_ASYNC, &(lu)->flags)
-#define SetLUAsync(lu) set_bit(LU_ASYNC, &(lu)->flags)
+#define LUWCache(lu) test_bit(LU_WCACHE, &(lu)->flags)
+#define SetLUWCache(lu) set_bit(LU_WCACHE, &(lu)->flags)
+#define ClearLUWCache(lu) clear_bit(LU_WCACHE, &(lu)->flags)
+
+#define LURCache(lu) test_bit(LU_RCACHE, &(lu)->flags)
+#define SetLURCache(lu) set_bit(LU_RCACHE, &(lu)->flags)
+#define ClearLURCache(lu) clear_bit(LU_RCACHE, &(lu)->flags)
 
 #define IET_HASH_ORDER 8
 #define cmnd_hashfn(itt) hash_long((itt), IET_HASH_ORDER)
diff --git a/kernel/target_disk.c b/kernel/target_disk.c
index 6e174fd..ba91380 100644
--- a/kernel/target_disk.c
+++ b/kernel/target_disk.c
@@ -22,15 +22,18 @@ static int insert_disconnect_pg(u8 *ptr)
  return sizeof(disconnect_pg);
 }
 
-static int insert_caching_pg(u8 *ptr, int async)
+static int insert_caching_pg(u8 *ptr, int wcache, int rcache)
 {
  unsigned char caching_pg[] = {0x08, 0x12, 0x10, 0x00, 0xff, 0xff, 0x00, 0x00,
       0xff, 0xff, 0xff, 0xff, 0x80, 0x14, 0x00, 0x00,
       0x00, 0x00, 0x00, 0x00};
 
  memcpy(ptr, caching_pg, sizeof(caching_pg));
- if (async)
+ if (wcache)
  ptr[2] |= 0x04; /* set WCE bit if we're caching writes */
+ if (!rcache)
+ ptr[2] |= 0x01; /* Read Cache Disable */
+
  return sizeof(caching_pg);
 }
 
@@ -124,7 +127,8 @@ static int build_mode_sense_response(struct iscsi_cmnd *cmnd)
  len += insert_geo_m_pg(data + len, cmnd->lun->blk_cnt);
  break;
  case 0x8:
- len += insert_caching_pg(data + len, LUAsync(cmnd->lun));
+ len += insert_caching_pg(data + len, LUWCache(cmnd->lun),
+ LURCache(cmnd->lun));
  break;
  case 0xa:
  len += insert_ctrl_m_pg(data + len);
@@ -136,7 +140,8 @@ static int build_mode_sense_response(struct iscsi_cmnd *cmnd)
  len += insert_disconnect_pg(data + len);
  len += insert_format_m_pg(data + len);
  len += insert_geo_m_pg(data + len, cmnd->lun->blk_cnt);
- len += insert_caching_pg(data + len, LUAsync(cmnd->lun));
+ len += insert_caching_pg(data + len, LUWCache(cmnd->lun),
+ LURCache(cmnd->lun));
  len += insert_ctrl_m_pg(data + len);
  len += insert_iec_m_pg(data + len);
  break;
@@ -370,7 +375,7 @@ static int build_write_response(struct iscsi_cmnd *cmnd)
 
  list_del_init(&cmnd->list);
  err = tio_write(cmnd->lun, tio);
- if (!err && !LUAsync(cmnd->lun))
+ if (!err && !LUWCache(cmnd->lun))
  err = tio_sync(cmnd->lun, tio);
 
  return err;
diff --git a/kernel/volume.c b/kernel/volume.c
index b807035..8a89aa5 100644
--- a/kernel/volume.c
+++ b/kernel/volume.c
@@ -63,7 +63,7 @@ static int set_iotype(struct iet_volume *volume, char *params)
  if (argp && !strcmp(argp, "ro"))
  SetLUReadonly(volume);
  else if (argp && !strcmp(argp, "wb"))
- SetLUAsync(volume);
+ SetLUWCache(volume);
  kfree(argp);
  break;
  default:
@@ -232,10 +232,11 @@ static void iet_volume_info_show(struct seq_file *seq, struct iscsi_target *targ
    volume->lun, volume->l_state, volume->iotype->name);
  if (LUReadonly(volume))
  seq_printf(seq, " iomode:ro");
- else if (LUAsync(volume))
+ else if (LUWCache(volume))
  seq_printf(seq, " iomode:wb");
  else
  seq_printf(seq, " iomode:wt");
+
  if (volume->iotype->show)
  volume->iotype->show(volume, seq);
  else
--
1.5.5




-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Iscsitarget-devel mailing list
Iscsitarget-devel@...
https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel

[FUJITA Tomonori]

On Thu, 19 Jun 2008 14:04:30 +0200
Arne Redlich <agr@...> wrote:

> Tomo,
>
> I previously posted this patch as RFC [1]. The feedback so far was
> positive [2] (thank you, Ross), so I'd like to request its
> inclusion unless you see any issues with it.

looks fine. Applied, thanks.

BTW, please don't send utf-8 charset mails.

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Iscsitarget-devel mailing list
Iscsitarget-devel@...
https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel

[Vladislav Bolkhovitin]

Arne Redlich wrote:
> ---
> Tomo,
>
> I previously posted this patch as RFC [1]. The feedback so far was
> positive [2] (thank you, Ross), so I'd like to request its
> inclusion unless you see any issues with it.

Just small note. Nowadays *all* disk drives come with write back caching
enabled. Moreover, on many of them it is impossible to turn it off,
although they can deceive it.

So, the assumption that BLOCKIO doesn't have write back caching is
pretty much unsafe.

Vlad

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Iscsitarget-devel mailing list
Iscsitarget-devel@...
https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel

[Arne Redlich-2]

FUJITA Tomonori <fujita.tomonori@...> writes:

> On Thu, 19 Jun 2008 14:04:30 +0200
> Arne Redlich <agr@...> wrote:
>
>> Tomo,
>>
>> I previously posted this patch as RFC [1]. The feedback so far was
>> positive [2] (thank you, Ross), so I'd like to request its
>> inclusion unless you see any issues with it.
>
> looks fine. Applied, thanks.

Thank you. But it appears that the commit is polluted by a compat
patch (patches/compat-2.6.22-2.6.23.patch). Can you please fix this?

> BTW, please don't send utf-8 charset mails.

Sorry, I used a different, obviously misconfigured MUA for this patch
than the usual one.

Cheers,
Arne

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Iscsitarget-devel mailing list
Iscsitarget-devel@...
https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel

[Arne Redlich-2]

Vladislav Bolkhovitin <vst@...> writes:

> Arne Redlich wrote:
>> ---
>> Tomo,
>>
>> I previously posted this patch as RFC [1]. The feedback so far was
>> positive [2] (thank you, Ross), so I'd like to request its
>> inclusion unless you see any issues with it.
>
> Just small note. Nowadays *all* disk drives come with write back caching
> enabled. Moreover, on many of them it is impossible to turn it off,
> although they can deceive it.
>
> So, the assumption that BLOCKIO doesn't have write back caching is
> pretty much unsafe.

Yes, that's what I noted in the initial submission of this patch [1]. But
this patch doesn't change blockio's WCE setting IIRC, only the RCD bit.

SCST lets the user specify the cache policy explicitly, right?

We should probably at least add a note to our documentation -
volunteers?

Cheers,
Arne

[1] http://article.gmane.org/gmane.linux.iscsi.iscsi-target.devel/8545

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Iscsitarget-devel mailing list
Iscsitarget-devel@...
https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel

[Vladislav Bolkhovitin]

Arne Redlich wrote:

> Vladislav Bolkhovitin <vst@...> writes:
>
>> Arne Redlich wrote:
>>> ---
>>> Tomo,
>>>
>>> I previously posted this patch as RFC [1]. The feedback so far was
>>> positive [2] (thank you, Ross), so I'd like to request its
>>> inclusion unless you see any issues with it.
>> Just small note. Nowadays *all* disk drives come with write back caching
>> enabled. Moreover, on many of them it is impossible to turn it off,
>> although they can deceive it.
>>
>> So, the assumption that BLOCKIO doesn't have write back caching is
>> pretty much unsafe.
>
> Yes, that's what I noted in the initial submission of this patch [1]. But
> this patch doesn't change blockio's WCE setting IIRC, only the RCD bit.
>
> SCST lets the user specify the cache policy explicitly, right?

Yes, via NV_CACHE option.

> We should probably at least add a note to our documentation -
> volunteers?
>
> Cheers,
> Arne
>
> [1] http://article.gmane.org/gmane.linux.iscsi.iscsi-target.devel/8545
>


-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Iscsitarget-devel mailing list
Iscsitarget-devel@...
https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel

[FUJITA Tomonori]

On Wed, 25 Jun 2008 11:59:57 +0200
Arne Redlich <agr@...> wrote:

> FUJITA Tomonori <fujita.tomonori@...> writes:
>
> > On Thu, 19 Jun 2008 14:04:30 +0200
> > Arne Redlich <agr@...> wrote:
> >
> >> Tomo,
> >>
> >> I previously posted this patch as RFC [1]. The feedback so far was
> >> positive [2] (thank you, Ross), so I'd like to request its
> >> inclusion unless you see any issues with it.
> >
> > looks fine. Applied, thanks.
>
> Thank you. But it appears that the commit is polluted by a compat
> patch (patches/compat-2.6.22-2.6.23.patch). Can you please fix this?

Sorry about that. It should be fixed now.

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Iscsitarget-devel mailing list
Iscsitarget-devel@...
https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel