Hi Martin, On Tue, 2015-02-10 at 17:27 +0100, Martin Svec wrote: > Hello, > > we have an issue with a higher number of Hyper-V hosts using one APTPL-enabled LIO iSCSI target LUN. > In particular, it's not possible to add more than 6 Hyper-V hosts to CSV cluster and target dmesg > shows the following errors: > > [60451129.483580] Unable to update renaming APTPL metadata > [60451129.483585] Unknown transport error for CDB 0x5f: 0 > [60451129.483948] Unable to update renaming APTPL metadata > [60451129.483952] Unknown transport error for CDB 0x5f: 0 > > Looking into reservations code, I guess that we're hitting PR_APTPL_BUF_LEN buffer size. With 6 > hosts and 4 iSCSI sessions from every initiator, the /var/target/pr/aptpl file size is very close to > the hardcoded 8192 bytes and there's no room for another host. When we reduce the number of active > sessions of some hosts to 2, it's possible to add the 7th host. > > Note that we automatically enforce aptpl=1 for all PRs in target_scsi3_emulate_pr_out() by a custom > patch. Without enforced APTPL, we observed random CSV stability issues during LIO HA target > failovers and other maintenance processes. Would the problem be related to it? > Yes, given the ring buffer output that makes sense. Thanks for the heads up. > Otherwise, can we workaround it just by increasing the PR_APTPL_BUF_LEN to e.g. 80 kB or is it a > more complex problem? > > We use kernels 3.7.10+ and 3.15.10+, but the 8 kB buffer limit seems to be in latest code too. > So I'd prefer to move this to vzalloc'd memory with a larger default value (256k), and add the ability to retry with a larger value (by 2x) if the expected APTPL output exceeds available buffer size. Care the test the following patch on your setup..? Note this is currently leaving the default PR_APTPL_BUF_LEN (8k) as is, but should automatically retry up to the required limit. Thank you, --nab diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c index d56f2aa..67fb3bf 100644 --- a/drivers/target/target_core_pr.c +++ b/drivers/target/target_core_pr.c @@ -1862,8 +1862,8 @@ static int core_scsi3_update_aptpl_buf( } if ((len + strlen(tmp) >= pr_aptpl_buf_len)) { - pr_err("Unable to update renaming" - " APTPL metadata\n"); + pr_err("Unable to update renaming APTPL metadata," + " reallocating larger buffer\n"); ret = -EMSGSIZE; goto out; } @@ -1880,8 +1880,8 @@ static int core_scsi3_update_aptpl_buf( lun->lun_sep->sep_rtpi, lun->unpacked_lun, reg_count); if ((len + strlen(tmp) >= pr_aptpl_buf_len)) { - pr_err("Unable to update renaming" - " APTPL metadata\n"); + pr_err("Unable to update renaming APTPL metadata," + " reallocating larger buffer\n"); ret = -EMSGSIZE; goto out; } @@ -1944,7 +1944,7 @@ static int __core_scsi3_write_aptpl_to_file( static sense_reason_t core_scsi3_update_and_write_aptpl(struct se_device *dev, bool aptpl) { unsigned char *buf; - int rc; + int rc, len = PR_APTPL_BUF_LEN; if (!aptpl) { char *null_buf = "No Registrations or Reservations\n"; @@ -1958,25 +1958,26 @@ static sense_reason_t core_scsi3_update_and_write_aptpl(struct se_device *dev, b return 0; } - - buf = kzalloc(PR_APTPL_BUF_LEN, GFP_KERNEL); +retry: + buf = vzalloc(len); if (!buf) return TCM_OUT_OF_RESOURCES; - rc = core_scsi3_update_aptpl_buf(dev, buf, PR_APTPL_BUF_LEN); + rc = core_scsi3_update_aptpl_buf(dev, buf, len); if (rc < 0) { - kfree(buf); - return TCM_OUT_OF_RESOURCES; + vfree(buf); + len *= 2; + goto retry; } rc = __core_scsi3_write_aptpl_to_file(dev, buf); if (rc != 0) { pr_err("SPC-3 PR: Could not update APTPL\n"); - kfree(buf); + vfree(buf); return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE; } dev->t10_pr.pr_aptpl_active = 1; - kfree(buf); + vfree(buf); pr_debug("SPC-3 PR: Set APTPL Bit Activated\n"); return 0; } -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html