Commit 459f213ba162bd13e113d6f92a8fa6c780fd67ed

Authored by Andy Grover
Committed by Nicholas Bellinger
1 parent 63e03349f5

target: Allocate aptpl_buf inside update_and_write_aptpl()

Instead of taking the buffer and length, update_and_write_aptpl() will
allocate the buffer as needed, and then free it. Instead, the function
takes an 'aptpl' boolean parameter.

This enables us to remove memory alloc/frees from struct
t10_pr_registration and other spots.

There is a slight loss of functionality because each callsite doesn't get
its own pr_debug any more, but this info can be cleaned via ftrace if
necessary and I think the shorter code is worth it.

Signed-off-by: Andy Grover <agrover@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>

Showing 2 changed files with 48 additions and 157 deletions Side-by-side Diff

drivers/target/target_core_pr.c
... ... @@ -606,13 +606,6 @@
606 606 return NULL;
607 607 }
608 608  
609   - pr_reg->pr_aptpl_buf = kzalloc(PR_APTPL_BUF_LEN, GFP_ATOMIC);
610   - if (!pr_reg->pr_aptpl_buf) {
611   - pr_err("Unable to allocate pr_reg->pr_aptpl_buf\n");
612   - kmem_cache_free(t10_pr_reg_cache, pr_reg);
613   - return NULL;
614   - }
615   -
616 609 INIT_LIST_HEAD(&pr_reg->pr_reg_list);
617 610 INIT_LIST_HEAD(&pr_reg->pr_reg_abort_list);
618 611 INIT_LIST_HEAD(&pr_reg->pr_reg_aptpl_list);
... ... @@ -803,7 +796,6 @@
803 796 pr_err("Unable to allocate struct t10_pr_registration\n");
804 797 return -ENOMEM;
805 798 }
806   - pr_reg->pr_aptpl_buf = kzalloc(PR_APTPL_BUF_LEN, GFP_KERNEL);
807 799  
808 800 INIT_LIST_HEAD(&pr_reg->pr_reg_list);
809 801 INIT_LIST_HEAD(&pr_reg->pr_reg_abort_list);
... ... @@ -1272,7 +1264,6 @@
1272 1264 if (!preempt_and_abort_list) {
1273 1265 pr_reg->pr_reg_deve = NULL;
1274 1266 pr_reg->pr_reg_nacl = NULL;
1275   - kfree(pr_reg->pr_aptpl_buf);
1276 1267 kmem_cache_free(t10_pr_reg_cache, pr_reg);
1277 1268 return;
1278 1269 }
... ... @@ -1341,7 +1332,6 @@
1341 1332 list_for_each_entry_safe(pr_reg, pr_reg_tmp, &pr_tmpl->aptpl_reg_list,
1342 1333 pr_reg_aptpl_list) {
1343 1334 list_del(&pr_reg->pr_reg_aptpl_list);
1344   - kfree(pr_reg->pr_aptpl_buf);
1345 1335 kmem_cache_free(t10_pr_reg_cache, pr_reg);
1346 1336 }
1347 1337 spin_unlock(&pr_tmpl->aptpl_reg_lock);
... ... @@ -1814,7 +1804,6 @@
1814 1804 kmem_cache_free(t10_pr_reg_cache, pr_reg_tmp);
1815 1805 }
1816 1806  
1817   - kfree(dest_pr_reg->pr_aptpl_buf);
1818 1807 kmem_cache_free(t10_pr_reg_cache, dest_pr_reg);
1819 1808  
1820 1809 if (dest_local_nexus)
... ... @@ -1840,8 +1829,6 @@
1840 1829 int reg_count = 0;
1841 1830 int ret = 0;
1842 1831  
1843   - memset(buf, 0, pr_aptpl_buf_len);
1844   -
1845 1832 spin_lock(&dev->dev_reservation_lock);
1846 1833 spin_lock(&dev->t10_pr.registration_lock);
1847 1834 /*
1848 1835  
1849 1836  
1850 1837  
1851 1838  
1852 1839  
... ... @@ -1965,31 +1952,45 @@
1965 1952 return ret ? -EIO : 0;
1966 1953 }
1967 1954  
1968   -static int
1969   -core_scsi3_update_and_write_aptpl(struct se_device *dev, unsigned char *in_buf,
1970   - u32 in_pr_aptpl_buf_len)
  1955 +/*
  1956 + * Clear the APTPL metadata if APTPL has been disabled, otherwise
  1957 + * write out the updated metadata to struct file for this SCSI device.
  1958 + */
  1959 +static int core_scsi3_update_and_write_aptpl(struct se_device *dev, bool aptpl)
1971 1960 {
1972   - unsigned char null_buf[64], *buf;
1973   - int ret;
  1961 + int ret = 0;
1974 1962  
1975   - /*
1976   - * Can be called with a NULL pointer from PROUT service action CLEAR
1977   - */
1978   - if (!in_buf) {
1979   - snprintf(null_buf, 64, "No Registrations or Reservations\n");
1980   - buf = null_buf;
  1963 + if (!aptpl) {
  1964 + char *null_buf = "No Registrations or Reservations\n";
  1965 +
  1966 + ret = __core_scsi3_write_aptpl_to_file(dev, null_buf);
  1967 + dev->t10_pr.pr_aptpl_active = 0;
  1968 + pr_debug("SPC-3 PR: Set APTPL Bit Deactivated\n");
1981 1969 } else {
1982   - ret = core_scsi3_update_aptpl_buf(dev, in_buf, in_pr_aptpl_buf_len);
1983   - if (ret != 0)
  1970 + int ret;
  1971 + unsigned char *buf;
  1972 +
  1973 + buf = kzalloc(PR_APTPL_BUF_LEN, GFP_KERNEL);
  1974 + if (!buf)
  1975 + return -ENOMEM;
  1976 +
  1977 + ret = core_scsi3_update_aptpl_buf(dev, buf, PR_APTPL_BUF_LEN);
  1978 + if (ret < 0) {
  1979 + kfree(buf);
1984 1980 return ret;
1985   - buf = in_buf;
  1981 + }
  1982 +
  1983 + ret = __core_scsi3_write_aptpl_to_file(dev, buf);
  1984 + if (ret != 0) {
  1985 + pr_err("SPC-3 PR: Could not update APTPL\n");
  1986 + } else {
  1987 + dev->t10_pr.pr_aptpl_active = 1;
  1988 + pr_debug("SPC-3 PR: Set APTPL Bit Activated\n");
  1989 + }
  1990 + kfree(buf);
1986 1991 }
1987 1992  
1988   - /*
1989   - * __core_scsi3_write_aptpl_to_file() will call strlen()
1990   - * on the passed buf to determine pr_aptpl_buf_len.
1991   - */
1992   - return __core_scsi3_write_aptpl_to_file(dev, buf);
  1993 + return ret;
1993 1994 }
1994 1995  
1995 1996 static sense_reason_t
... ... @@ -2003,8 +2004,6 @@
2003 2004 struct se_portal_group *se_tpg;
2004 2005 struct t10_pr_registration *pr_reg, *pr_reg_p, *pr_reg_tmp, *pr_reg_e;
2005 2006 struct t10_reservation *pr_tmpl = &dev->t10_pr;
2006   - /* Used for APTPL metadata w/ UNREGISTER */
2007   - unsigned char *pr_aptpl_buf = NULL;
2008 2007 unsigned char isid_buf[PR_REG_ISID_LEN], *isid_ptr = NULL;
2009 2008 sense_reason_t ret = TCM_NO_SENSE;
2010 2009 int pr_holder = 0, type;
2011 2010  
... ... @@ -2066,31 +2065,8 @@
2066 2065 if (ret != 0)
2067 2066 return ret;
2068 2067 }
2069   - /*
2070   - * Nothing left to do for the APTPL=0 case.
2071   - */
2072   - if (!aptpl) {
2073   - pr_tmpl->pr_aptpl_active = 0;
2074   - core_scsi3_update_and_write_aptpl(cmd->se_dev, NULL, 0);
2075   - pr_debug("SPC-3 PR: Set APTPL Bit Deactivated for"
2076   - " REGISTER\n");
2077   - return 0;
2078   - }
2079   - /*
2080   - * Locate the newly allocated local I_T Nexus *pr_reg, and
2081   - * update the APTPL metadata information using its
2082   - * preallocated *pr_reg->pr_aptpl_buf.
2083   - */
2084   - pr_reg = core_scsi3_locate_pr_reg(cmd->se_dev,
2085   - se_sess->se_node_acl, se_sess);
2086 2068  
2087   - if (core_scsi3_update_and_write_aptpl(cmd->se_dev,
2088   - pr_reg->pr_aptpl_buf, PR_APTPL_BUF_LEN)) {
2089   - pr_tmpl->pr_aptpl_active = 1;
2090   - pr_debug("SPC-3 PR: Set APTPL Bit Activated for REGISTER\n");
2091   - }
2092   -
2093   - goto out_put_pr_reg;
  2069 + return core_scsi3_update_and_write_aptpl(dev, aptpl);
2094 2070 }
2095 2071  
2096 2072 /*
... ... @@ -2131,19 +2107,6 @@
2131 2107 }
2132 2108  
2133 2109 /*
2134   - * Allocate APTPL metadata buffer used for UNREGISTER ops
2135   - */
2136   - if (aptpl) {
2137   - pr_aptpl_buf = kzalloc(PR_APTPL_BUF_LEN, GFP_KERNEL);
2138   - if (!pr_aptpl_buf) {
2139   - pr_err("Unable to allocate"
2140   - " pr_aptpl_buf\n");
2141   - ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
2142   - goto out_put_pr_reg;
2143   - }
2144   - }
2145   -
2146   - /*
2147 2110 * sa_res_key=0 Unregister Reservation Key for registered I_T
2148 2111 * Nexus sa_res_key=1 Change Reservation Key for registered I_T
2149 2112 * Nexus.
... ... @@ -2152,7 +2115,6 @@
2152 2115 pr_holder = core_scsi3_check_implict_release(
2153 2116 cmd->se_dev, pr_reg);
2154 2117 if (pr_holder < 0) {
2155   - kfree(pr_aptpl_buf);
2156 2118 ret = TCM_RESERVATION_CONFLICT;
2157 2119 goto out_put_pr_reg;
2158 2120 }
... ... @@ -2214,21 +2176,8 @@
2214 2176 }
2215 2177 spin_unlock(&pr_tmpl->registration_lock);
2216 2178  
2217   - if (!aptpl) {
2218   - pr_tmpl->pr_aptpl_active = 0;
2219   - core_scsi3_update_and_write_aptpl(dev, NULL, 0);
2220   - pr_debug("SPC-3 PR: Set APTPL Bit Deactivated"
2221   - " for UNREGISTER\n");
2222   - return 0;
2223   - }
2224   -
2225   - if (!core_scsi3_update_and_write_aptpl(dev, pr_aptpl_buf, PR_APTPL_BUF_LEN)) {
2226   - pr_tmpl->pr_aptpl_active = 1;
2227   - pr_debug("SPC-3 PR: Set APTPL Bit Activated"
2228   - " for UNREGISTER\n");
2229   - }
2230   -
2231   - goto out_free_aptpl_buf;
  2179 + ret = core_scsi3_update_and_write_aptpl(dev, aptpl);
  2180 + goto out_put_pr_reg;
2232 2181 }
2233 2182  
2234 2183 /*
2235 2184  
... ... @@ -2245,24 +2194,8 @@
2245 2194 pr_reg->pr_reg_nacl->initiatorname,
2246 2195 pr_reg->pr_res_key, pr_reg->pr_res_generation);
2247 2196  
2248   - if (!aptpl) {
2249   - pr_tmpl->pr_aptpl_active = 0;
2250   - core_scsi3_update_and_write_aptpl(dev, NULL, 0);
2251   - pr_debug("SPC-3 PR: Set APTPL Bit Deactivated"
2252   - " for REGISTER\n");
2253   - ret = 0;
2254   - goto out_put_pr_reg;
2255   - }
  2197 + ret = core_scsi3_update_and_write_aptpl(dev, aptpl);
2256 2198  
2257   - if (!core_scsi3_update_and_write_aptpl(dev, pr_aptpl_buf, PR_APTPL_BUF_LEN)) {
2258   - pr_tmpl->pr_aptpl_active = 1;
2259   - pr_debug("SPC-3 PR: Set APTPL Bit Activated"
2260   - " for REGISTER\n");
2261   - }
2262   -
2263   -out_free_aptpl_buf:
2264   - kfree(pr_aptpl_buf);
2265   - ret = 0;
2266 2199 out_put_pr_reg:
2267 2200 core_scsi3_put_pr_reg(pr_reg);
2268 2201 return ret;
... ... @@ -2437,13 +2370,8 @@
2437 2370 i_buf);
2438 2371 spin_unlock(&dev->dev_reservation_lock);
2439 2372  
2440   - if (pr_tmpl->pr_aptpl_active) {
2441   - if (!core_scsi3_update_and_write_aptpl(cmd->se_dev,
2442   - pr_reg->pr_aptpl_buf, PR_APTPL_BUF_LEN)) {
2443   - pr_debug("SPC-3 PR: Updated APTPL metadata"
2444   - " for RESERVE\n");
2445   - }
2446   - }
  2373 + if (pr_tmpl->pr_aptpl_active)
  2374 + core_scsi3_update_and_write_aptpl(cmd->se_dev, true);
2447 2375  
2448 2376 ret = 0;
2449 2377 out_put_pr_reg:
... ... @@ -2657,12 +2585,9 @@
2657 2585 spin_unlock(&pr_tmpl->registration_lock);
2658 2586  
2659 2587 write_aptpl:
2660   - if (pr_tmpl->pr_aptpl_active) {
2661   - if (!core_scsi3_update_and_write_aptpl(cmd->se_dev,
2662   - pr_reg->pr_aptpl_buf, PR_APTPL_BUF_LEN)) {
2663   - pr_debug("SPC-3 PR: Updated APTPL metadata for RELEASE\n");
2664   - }
2665   - }
  2588 + if (pr_tmpl->pr_aptpl_active)
  2589 + core_scsi3_update_and_write_aptpl(cmd->se_dev, true);
  2590 +
2666 2591 out_put_pr_reg:
2667 2592 core_scsi3_put_pr_reg(pr_reg);
2668 2593 return ret;
... ... @@ -2746,11 +2671,7 @@
2746 2671 pr_debug("SPC-3 PR [%s] Service Action: CLEAR complete\n",
2747 2672 cmd->se_tfo->get_fabric_name());
2748 2673  
2749   - if (pr_tmpl->pr_aptpl_active) {
2750   - core_scsi3_update_and_write_aptpl(cmd->se_dev, NULL, 0);
2751   - pr_debug("SPC-3 PR: Updated APTPL metadata"
2752   - " for CLEAR\n");
2753   - }
  2674 + core_scsi3_update_and_write_aptpl(cmd->se_dev, false);
2754 2675  
2755 2676 core_scsi3_pr_generation(dev);
2756 2677 return 0;
... ... @@ -2822,7 +2743,6 @@
2822 2743  
2823 2744 pr_reg->pr_reg_deve = NULL;
2824 2745 pr_reg->pr_reg_nacl = NULL;
2825   - kfree(pr_reg->pr_aptpl_buf);
2826 2746 kmem_cache_free(t10_pr_reg_cache, pr_reg);
2827 2747 }
2828 2748 }
... ... @@ -2984,14 +2904,8 @@
2984 2904 }
2985 2905 spin_unlock(&dev->dev_reservation_lock);
2986 2906  
2987   - if (pr_tmpl->pr_aptpl_active) {
2988   - if (!core_scsi3_update_and_write_aptpl(cmd->se_dev,
2989   - pr_reg_n->pr_aptpl_buf, PR_APTPL_BUF_LEN)) {
2990   - pr_debug("SPC-3 PR: Updated APTPL"
2991   - " metadata for PREEMPT%s\n", (preempt_type == PREEMPT_AND_ABORT) ?
2992   - "_AND_ABORT" : "");
2993   - }
2994   - }
  2907 + if (pr_tmpl->pr_aptpl_active)
  2908 + core_scsi3_update_and_write_aptpl(cmd->se_dev, true);
2995 2909  
2996 2910 core_scsi3_put_pr_reg(pr_reg_n);
2997 2911 core_scsi3_pr_generation(cmd->se_dev);
... ... @@ -3119,13 +3033,8 @@
3119 3033 pr_reg_n);
3120 3034 }
3121 3035  
3122   - if (pr_tmpl->pr_aptpl_active) {
3123   - if (!core_scsi3_update_and_write_aptpl(cmd->se_dev,
3124   - pr_reg_n->pr_aptpl_buf, PR_APTPL_BUF_LEN)) {
3125   - pr_debug("SPC-3 PR: Updated APTPL metadata for PREEMPT"
3126   - "%s\n", (preempt_type == PREEMPT_AND_ABORT) ? "_AND_ABORT" : "");
3127   - }
3128   - }
  3036 + if (pr_tmpl->pr_aptpl_active)
  3037 + core_scsi3_update_and_write_aptpl(cmd->se_dev, true);
3129 3038  
3130 3039 core_scsi3_put_pr_reg(pr_reg_n);
3131 3040 core_scsi3_pr_generation(cmd->se_dev);
... ... @@ -3552,23 +3461,7 @@
3552 3461 } else
3553 3462 core_scsi3_put_pr_reg(pr_reg);
3554 3463  
3555   - /*
3556   - * Clear the APTPL metadata if APTPL has been disabled, otherwise
3557   - * write out the updated metadata to struct file for this SCSI device.
3558   - */
3559   - if (!aptpl) {
3560   - pr_tmpl->pr_aptpl_active = 0;
3561   - core_scsi3_update_and_write_aptpl(cmd->se_dev, NULL, 0);
3562   - pr_debug("SPC-3 PR: Set APTPL Bit Deactivated for"
3563   - " REGISTER_AND_MOVE\n");
3564   - } else {
3565   - pr_tmpl->pr_aptpl_active = 1;
3566   - if (!core_scsi3_update_and_write_aptpl(cmd->se_dev,
3567   - dest_pr_reg->pr_aptpl_buf, PR_APTPL_BUF_LEN)) {
3568   - pr_debug("SPC-3 PR: Set APTPL Bit Activated for"
3569   - " REGISTER_AND_MOVE\n");
3570   - }
3571   - }
  3464 + core_scsi3_update_and_write_aptpl(cmd->se_dev, aptpl);
3572 3465  
3573 3466 transport_kunmap_data_sg(cmd);
3574 3467  
include/target/target_core_base.h
... ... @@ -339,8 +339,6 @@
339 339 /* Used during APTPL metadata reading */
340 340 #define PR_APTPL_MAX_TPORT_LEN 256
341 341 unsigned char pr_tport[PR_APTPL_MAX_TPORT_LEN];
342   - /* For writing out live meta data */
343   - unsigned char *pr_aptpl_buf;
344 342 u16 pr_aptpl_rpti;
345 343 u16 pr_reg_tpgt;
346 344 /* Reservation effects all target ports */