[lustre-devel] [PATCH 00/30] lustre: first batch of fixes from lustre 2.10

[lustre-devel] [PATCH 00/30] lustre: first batch of fixes from lustre 2.10

[James Simmons]

First collection of patches to address various cleanups and bugs in the
lustre 2.10 release that are missing for the lustre linux client. These
patches can be applied in any order.

Alexander Boyko (2):
  lustre: osc: fix for cl_env_get in low memory
  lustre: ptlrpc: fix wrong error handlers

Alexandr Boyko (1):
  lustre: osc: adds radix_tree_preload

Andreas Dilger (2):
  lustre: llog: update llog print format to use FIDs
  lustre: mdc: allow setting readdir RPC size parameter

Andrew Perepechko (1):
  lustre: llite: llite.stat_blocksize param for fixed st_blksize

Andriy Skulysh (3):
  lustre: osc: GPF while doing ELC with no_wait_policy
  lustre: ldlm: GPF in _ldlm_lock_debug()
  lustre: llite: protect ll_dentry_data modification

Bobi Jam (2):
  lustre: osc: osc page lru list race
  lustre: llite: check the return value of cl_file_inode_init()

Bruno Faccini (1):
  lustre: ptlrpc: drain "ptlrpc_request_buffer_desc" objects

Doug Oucharek (1):
  lustre: lnet: Change sock_create() to sock_create_kern()

Fan Yong (1):
  lustre: ptlrpc: free reply buffer earlier for open RPC

James Simmons (2):
  lustre: lnd: resolve IP query code in LND drivers
  lustre: clio: fix spare bit handling

John L. Hammond (3):
  lustre: ptlrpc: set rq_sent when send fails due to -ENOMEM
  lustre: kuc: initialize kkuc_groups at module init time
  lustre: ldlm: use static initializer macros where possible

Niu Yawei (3):
  lustre: uapi: add documentation about FIDs
  lustre: osc: update timestamps on write only
  lustre: ptlrpc: add replay request into unreplied list

Patrick Farrell (1):
  lustre: ldlm: cond_resched in ldlm_bl_thread_main

Sebastien Buisson (1):
  lustre: llite: set sec ctx on client's inode at create time

Steve Guminski (3):
  lustre: ptlrpc: Prevent possible dereference of NULL pointers
  lustre: mgc: Remove unnecessary checks for config_log_put()
  lustre: llite: rcu-walk check should not depend on statahead

Vitaly Fertman (1):
  lustre: ptlrpc: increase sleep time in ptlrpc_request_bufs_pack()

Wang Shilong (1):
  lustre: obd: remove obsolete OBD_FL_LOCAL_MASK

Yang Sheng (1):
  lustre: libcfs: use save_stack_trace for stack dump

 .../lustre/include/linux/libcfs/libcfs_debug.h     |  1 +
 .../lustre/include/uapi/linux/lustre/lustre_idl.h  | 88 +++++++++++++++++++++-
 .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    | 12 ++-
 .../staging/lustre/lnet/klnds/socklnd/socklnd.c    |  2 +-
 drivers/staging/lustre/lnet/libcfs/debug.c         | 46 ++++++++++-
 drivers/staging/lustre/lnet/lnet/lib-socket.c      |  2 +-
 .../staging/lustre/lustre/include/lprocfs_status.h |  5 ++
 drivers/staging/lustre/lustre/include/lustre_dlm.h |  4 +-
 .../lustre/lustre/include/lustre_kernelcomm.h      |  1 +
 drivers/staging/lustre/lustre/include/lustre_net.h | 14 ++--
 drivers/staging/lustre/lustre/include/obd.h        |  5 +-
 drivers/staging/lustre/lustre/ldlm/ldlm_lock.c     | 13 +++-
 drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c    | 12 +--
 drivers/staging/lustre/lustre/ldlm/ldlm_resource.c |  6 +-
 drivers/staging/lustre/lustre/llite/dcache.c       |  7 +-
 drivers/staging/lustre/lustre/llite/dir.c          | 22 +++---
 drivers/staging/lustre/lustre/llite/file.c         |  2 +-
 drivers/staging/lustre/lustre/llite/lcommon_cl.c   | 19 ++---
 .../staging/lustre/lustre/llite/llite_internal.h   |  8 +-
 drivers/staging/lustre/lustre/llite/llite_lib.c    | 24 +++---
 drivers/staging/lustre/lustre/llite/llite_nfs.c    |  4 +-
 drivers/staging/lustre/lustre/llite/lproc_llite.c  | 33 ++++++++
 drivers/staging/lustre/lustre/llite/namei.c        | 59 ++++++++++++---
 drivers/staging/lustre/lustre/llite/statahead.c    |  4 -
 drivers/staging/lustre/lustre/mdc/lproc_mdc.c      | 20 +----
 drivers/staging/lustre/lustre/mdc/mdc_request.c    | 35 ++++++---
 drivers/staging/lustre/lustre/mgc/mgc_request.c    | 36 ++++-----
 drivers/staging/lustre/lustre/obdclass/class_obd.c |  3 +
 .../staging/lustre/lustre/obdclass/kernelcomm.c    | 38 +++++++---
 drivers/staging/lustre/lustre/obdclass/llog_cat.c  | 28 +++----
 drivers/staging/lustre/lustre/obdclass/llog_swab.c |  4 +-
 .../lustre/lustre/obdclass/lprocfs_status.c        | 54 +++++++++++++
 drivers/staging/lustre/lustre/obdclass/obdo.c      | 11 ---
 drivers/staging/lustre/lustre/osc/lproc_osc.c      | 46 -----------
 drivers/staging/lustre/lustre/osc/osc_cache.c      |  3 +
 drivers/staging/lustre/lustre/osc/osc_io.c         |  2 +-
 drivers/staging/lustre/lustre/osc/osc_lock.c       | 13 +++-
 drivers/staging/lustre/lustre/osc/osc_object.c     |  2 +-
 drivers/staging/lustre/lustre/osc/osc_page.c       | 43 ++++++-----
 drivers/staging/lustre/lustre/osc/osc_request.c    |  7 +-
 drivers/staging/lustre/lustre/ptlrpc/client.c      | 67 +++++++++++-----
 drivers/staging/lustre/lustre/ptlrpc/niobuf.c      | 20 ++---
 .../staging/lustre/lustre/ptlrpc/pack_generic.c    | 26 +++++--
 .../staging/lustre/lustre/ptlrpc/ptlrpc_internal.h |  1 +
 drivers/staging/lustre/lustre/ptlrpc/recover.c     | 12 +--
 drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c    |  2 +-
 drivers/staging/lustre/lustre/ptlrpc/service.c     | 14 +++-
 drivers/staging/lustre/lustre/ptlrpc/wiretest.c    |  1 -
 48 files changed, 585 insertions(+), 296 deletions(-)

-- 
1.8.3.1

[lustre-devel] [PATCH 01/30] lustre: lnd: resolve IP query code in LND drivers

[James Simmons]

The recent IP querying code that landed to both ksocklnd and
ko2iblnd have some bugs that prevent proper setup. The first
bug in both drivers is that the IP address of ifa_local is in
big endian format so on little endian systems the IP address
is incorrect. Calling ntohl() on ifa_local gets the real IP
address. The second bug located in ko2iblnd is that in_dev is
always NULL. Add the call __in_dev_get_rtnl() to get in_dev
and use that information to query the IP address.

Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c | 12 +++++++++---
 drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c |  2 +-
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
index 15953e4..6874e53 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
@@ -2468,8 +2468,7 @@ static struct kib_dev *kiblnd_create_dev(char *ifname)
 	flags = dev_get_flags(netdev);
 	if (!(flags & IFF_UP)) {
 		CERROR("Can't query IPoIB interface %s: it's down\n", ifname);
-		rtnl_unlock();
-		return NULL;
+		goto unlock;
 	}
 
 	dev = kzalloc(sizeof(*dev), GFP_NOFS);
@@ -2481,9 +2480,16 @@ static struct kib_dev *kiblnd_create_dev(char *ifname)
 	INIT_LIST_HEAD(&dev->ibd_nets);
 	INIT_LIST_HEAD(&dev->ibd_list); /* not yet in kib_devs */
 	INIT_LIST_HEAD(&dev->ibd_fail_list);
+
+	in_dev = __in_dev_get_rtnl(netdev);
+	if (!in_dev) {
+		kfree(dev);
+		goto unlock;
+	}
+
 	for_primary_ifa(in_dev)
 		if (strcmp(ifa->ifa_label, ifname) == 0) {
-			dev->ibd_ifip = ifa->ifa_local;
+			dev->ibd_ifip = ntohl(ifa->ifa_local);
 			break;
 		}
 	endfor_ifa(in_dev);
diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
index 5b81040..750a7ce 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
@@ -2589,7 +2589,7 @@ static int ksocknal_push(struct lnet_ni *ni, struct lnet_process_id id)
 		}
 		for_primary_ifa(in_dev)
 			if (strcmp(ifa->ifa_label, name) == 0) {
-				ksi->ksni_ipaddr = ifa->ifa_local;
+				ksi->ksni_ipaddr = ntohl(ifa->ifa_local);
 				ksi->ksni_netmask = ifa->ifa_mask;
 				strlcpy(ksi->ksni_name,
 					name, sizeof(ksi->ksni_name));
-- 
1.8.3.1

[lustre-devel] [PATCH 02/30] lustre: uapi: add documentation about FIDs

[James Simmons]

From: Niu Yawei <yawei.niu@intel.com>

Add details about FIDs in lustre. FIDs are exposed to user land
so users can manage them. This provides the users details to
assist them.

Signed-off-by: Niu Yawei <yawei.niu@intel.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-8998
Reviewed-on: https://review.whamcloud.com/24867
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Jinshan Xiong <jinshan.xiong@gmail.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 .../lustre/include/uapi/linux/lustre/lustre_idl.h  | 84 ++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/drivers/staging/lustre/include/uapi/linux/lustre/lustre_idl.h b/drivers/staging/lustre/include/uapi/linux/lustre/lustre_idl.h
index bec1028..58321eb 100644
--- a/drivers/staging/lustre/include/uapi/linux/lustre/lustre_idl.h
+++ b/drivers/staging/lustre/include/uapi/linux/lustre/lustre_idl.h
@@ -223,6 +223,90 @@ enum {
 #define LL_HSM_MAX_ARCHIVE (sizeof(__u32) * 8)
 
 /**
+ * Different FID Format
+ * http://arch.lustre.org/index.php?title=Interoperability_fids_zfs#NEW.0
+ *
+ * FID:
+ * File IDentifier generated by client from range allocated by the seq service.
+ * First 0x400 sequences [2^33, 2^33 + 0x400] are reserved for system use. Note
+ * that on ldiskfs MDTs that IGIF FIDs can use inode numbers starting at 12,
+ * but this is in the IGIF SEQ rangeand does not conflict with assigned FIDs.
+ *
+ * IGIF:
+ * Inode and Generation In FID, a surrogate FID used to globally identify an
+ * existing object on OLD formatted MDT file system. This would only be used on
+ * MDT0 in a DNE filesystem, because there are not expected to be any OLD
+ * formatted DNE filesystems. Belongs to a sequence in [12, 2^32 - 1] range,
+ * where sequence number is inode number, and inode generation is used as OID.
+ * NOTE: This assumes no more than 2^32-1 inodes exist in the MDT filesystem,
+ * which is the maximum possible for an ldiskfs backend. NOTE: This assumes
+ * that the reserved ext3/ext4/ldiskfs inode numbers [0-11] are never visible
+ * to clients, which has always been true.
+ *
+ * IDIF:
+ * Object ID in FID, a surrogate FID used to globally identify an existing
+ * object on OLD formatted OST file system. Belongs to a sequence in
+ * [2^32, 2^33 - 1]. Sequence number is calculated as:
+ *     1 << 32 | (ost_index << 16) | ((objid >> 32) & 0xffff)
+ * that is, SEQ consists of 16-bit OST index, and higher 16 bits of object ID.
+ * The generation of unique SEQ values per OST allows the IDIF FIDs to be
+ * identified in the FLD correctly. The OID field is calculated as:
+ *     objid & 0xffffffff
+ * that is, it consists of lower 32 bits of object ID. NOTE This assumes that
+ * no more than 2^48-1 objects have ever been created on an OST, and that no
+ * more than 65535 OSTs are in use. Both are very reasonable assumptions (can
+ * uniquely map all objects on an OST that created 1M objects per second for 9
+ * years, or combinations thereof).
+ *
+ * OST_MDT0:
+ * Surrogate FID used to identify an existing object on OLD formatted OST
+ * filesystem. Belongs to the reserved sequence 0, and is used internally prior
+ * to the introduction of FID-on-OST, at which point IDIF will be used to
+ * identify objects as residing on a specific OST.
+ *
+ * LLOG:
+ * For Lustre Log objects the object sequence 1 is used. This is compatible with
+ * both OLD and NEW.1 namespaces, as this SEQ number is in the ext3/ldiskfs
+ * reserved inode range and does not conflict with IGIF sequence numbers.
+ *
+ * ECHO:
+ * For testing OST IO performance the object sequence 2 is used. This is
+ * compatible with both OLD and NEW.1 namespaces, as this SEQ number is in the
+ * ext3/ldiskfs reserved inode range and does not conflict with IGIF sequence
+ * numbers.
+ *
+ * OST_MDT1 .. OST_MAX:
+ * For testing with multiple MDTs the object sequence 3 through 9 is used,
+ * allowing direct mapping of MDTs 1 through 7 respectively, for a total of 8
+ * MDTs including OST_MDT0. This matches the legacy CMD project "group"
+ * mappings. However, this SEQ range is only for testing prior to any production
+ * DNE release, as the objects in this range conflict across all OSTs, as the
+ * OST index is not part of the FID.
+ *
+ *
+ * For compatibility with existing OLD OST network protocol structures, the FID
+ * must map onto the o_id and o_gr in a manner that ensures existing objects are
+ * identified consistently for IO, as well as onto the lock namespace to ensure
+ * both IDIFs map onto the same objects for IO as well as resources in the DLM.
+ *
+ * DLM OLD OBIF/IDIF:
+ * resource[] = {o_id, o_seq, 0, 0};  // o_seq == 0 for production releases
+ *
+ * DLM NEW.1 FID (this is the same for both the MDT and OST):
+ * resource[] = {SEQ, OID, VER, HASH};
+ *
+ * Note that for mapping IDIF values to DLM resource names the o_id may be
+ * larger than the 2^33 reserved sequence numbers for IDIF, so it is possible
+ * for the o_id numbers to overlap FID SEQ numbers in the resource. However, in
+ * all production releases the OLD o_seq field is always zero, and all valid FID
+ * OID values are non-zero, so the lock resources will not collide.
+ *
+ * For objects within the IDIF range, group extraction (non-CMD) will be:
+ * o_id = (fid->f_seq & 0x7fff) << 16 | fid->f_oid;
+ * o_seq = 0;  // formerly group number
+ */
+
+/**
  * Note that reserved SEQ numbers below 12 will conflict with ldiskfs
  * inodes in the IGIF namespace, so these reserved SEQ numbers can be
  * used for other purposes and not risk collisions with existing inodes.
-- 
1.8.3.1

[lustre-devel] [PATCH 03/30] lustre: osc: fix for cl_env_get in low memory

[James Simmons]

From: Alexander Boyko <c17825@cray.com>

In low memory situation cl_env_get->cl_env_new->kmem_cache_alloc
could fail with ENOMEM error. Some parts doesn`t handle error
case, for example:
...(osc_lock_upcall()) ASSERTION( !IS_ERR(env) ) failed:
...(osc_lock.c:315:osc_lock_upcall()) LBUG

For osc_lock_upcall() the patch changes cl_env_get to
cl_env_percpu_peek, this prevents memory allocation and
couldn`t fail in low memory case.
For osc_extent_truncate() the patch adds error handle.

Signed-off-by: Alexander Boyko <c17825@cray.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-9065
Seagate-bug-id: MRP-4120
Reviewed-on: https://review.whamcloud.com/25171
Reviewed-by: Jinshan Xiong <jinshan.xiong@gmail.com>
Reviewed-by: Andrew Perepechko <c17827@cray.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/osc/osc_cache.c | 3 +++
 drivers/staging/lustre/lustre/osc/osc_lock.c  | 5 ++---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/lustre/lustre/osc/osc_cache.c b/drivers/staging/lustre/lustre/osc/osc_cache.c
index e44822a..326f663 100644
--- a/drivers/staging/lustre/lustre/osc/osc_cache.c
+++ b/drivers/staging/lustre/lustre/osc/osc_cache.c
@@ -1038,6 +1038,9 @@ static int osc_extent_truncate(struct osc_extent *ext, pgoff_t trunc_index,
 	 * it's from lov_io_sub and not fully initialized.
 	 */
 	env = cl_env_get(&refcheck);
+	if (IS_ERR(env))
+		return PTR_ERR(env);
+
 	io  = &osc_env_info(env)->oti_io;
 	io->ci_obj = cl_object_top(osc2cl(obj));
 	io->ci_ignore_layout = 1;
diff --git a/drivers/staging/lustre/lustre/osc/osc_lock.c b/drivers/staging/lustre/lustre/osc/osc_lock.c
index d93d33d..d6a275f 100644
--- a/drivers/staging/lustre/lustre/osc/osc_lock.c
+++ b/drivers/staging/lustre/lustre/osc/osc_lock.c
@@ -297,9 +297,8 @@ static int osc_lock_upcall(void *cookie, struct lustre_handle *lockh,
 	struct cl_lock_slice *slice = &oscl->ols_cl;
 	struct lu_env *env;
 	int rc;
-	u16 refcheck;
 
-	env = cl_env_get(&refcheck);
+	env = cl_env_percpu_get();
 	/* should never happen, similar to osc_ldlm_blocking_ast(). */
 	LASSERT(!IS_ERR(env));
 
@@ -338,7 +337,7 @@ static int osc_lock_upcall(void *cookie, struct lustre_handle *lockh,
 
 	if (oscl->ols_owner)
 		cl_sync_io_note(env, oscl->ols_owner, rc);
-	cl_env_put(env, &refcheck);
+	cl_env_percpu_put(env);
 
 	return rc;
 }
-- 
1.8.3.1

[lustre-devel] [PATCH 04/30] lustre: ptlrpc: fix wrong error handlers

[James Simmons]

From: Alexander Boyko <c17825@cray.com>

If ptlrpc_request_pack finish with 0, we need to call
ptlrpc_req_finished for later errors.

Signed-off-by: Alexander Boyko <c17825@cray.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-9286
Seagate-bug-id: MRP-4285
Reviewed-on: https://review.whamcloud.com/26319
Reviewed-by: Chris Horn <hornc@cray.com>
Reviewed-by: Mike Pershin <mpershin@whamcloud.com>
Reviewed-by: Niu Yawei <yawei.niu@intel.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/mdc/mdc_request.c | 2 +-
 drivers/staging/lustre/lustre/osc/osc_request.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/mdc/mdc_request.c b/drivers/staging/lustre/lustre/mdc/mdc_request.c
index f3c0722..116e973 100644
--- a/drivers/staging/lustre/lustre/mdc/mdc_request.c
+++ b/drivers/staging/lustre/lustre/mdc/mdc_request.c
@@ -904,7 +904,7 @@ static int mdc_getpage(struct obd_export *exp, const struct lu_fid *fid,
 				    MDS_BULK_PORTAL,
 				    &ptlrpc_bulk_kiov_pin_ops);
 	if (!desc) {
-		ptlrpc_request_free(req);
+		ptlrpc_req_finished(req);
 		return -ENOMEM;
 	}
 
diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c b/drivers/staging/lustre/lustre/osc/osc_request.c
index 7c91c4b..8024b17 100644
--- a/drivers/staging/lustre/lustre/osc/osc_request.c
+++ b/drivers/staging/lustre/lustre/osc/osc_request.c
@@ -655,7 +655,7 @@ static int osc_destroy(const struct lu_env *env, struct obd_export *exp,
 		rc = l_wait_event_abortable_exclusive(cli->cl_destroy_waitq,
 						      osc_can_send_destroy(cli));
 		if (rc) {
-			ptlrpc_request_free(req);
+			ptlrpc_req_finished(req);
 			return rc;
 		}
 	}
-- 
1.8.3.1

[lustre-devel] [PATCH 05/30] lustre: osc: GPF while doing ELC with no_wait_policy

[James Simmons]

From: Andriy Skulysh <c17819@cray.com>

osc_ldlm_weigh_ast() uses osc_object from ldlm_lock
without taking a reference.

It should take a reference like osc_ldlm_glimpse_ast()/
osc_dlm_blocking_ast0() does.

Signed-off-by: Andriy Skulysh <c17819@cray.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-9171
Seagate-bug-id: MRP-4179
Reviewed-on: https://review.whamcloud.com/25700
Reviewed-by: Jinshan Xiong <jinshan.xiong@gmail.com>
Reviewed-by: Bobi Jam <bobijam@hotmail.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/osc/osc_lock.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/staging/lustre/lustre/osc/osc_lock.c b/drivers/staging/lustre/lustre/osc/osc_lock.c
index d6a275f..6059dba 100644
--- a/drivers/staging/lustre/lustre/osc/osc_lock.c
+++ b/drivers/staging/lustre/lustre/osc/osc_lock.c
@@ -701,7 +701,12 @@ unsigned long osc_ldlm_weigh_ast(struct ldlm_lock *dlmlock)
 		return 1;
 
 	LASSERT(dlmlock->l_resource->lr_type == LDLM_EXTENT);
+	lock_res_and_lock(dlmlock);
 	obj = dlmlock->l_ast_data;
+	if (obj)
+		cl_object_get(osc2cl(obj));
+	unlock_res_and_lock(dlmlock);
+
 	if (!obj) {
 		weight = 1;
 		goto out;
@@ -725,6 +730,9 @@ unsigned long osc_ldlm_weigh_ast(struct ldlm_lock *dlmlock)
 	weight = osc_lock_weight(env, obj, &dlmlock->l_policy_data.l_extent);
 
 out:
+	if (obj)
+		cl_object_put(env, osc2cl(obj));
+
 	cl_env_put(env, &refcheck);
 	return weight;
 }
-- 
1.8.3.1

[lustre-devel] [PATCH 06/30] lustre: ptlrpc: Prevent possible dereference of NULL pointers

[James Simmons]

From: Steve Guminski <stephenx.guminski@intel.com>

Check pointers for NULL before passing to other functions.

Signed-off-by: Steve Guminski <stephenx.guminski@intel.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-9076
Reviewed-on: https://review.whamcloud.com/25909
Reviewed-by: John L. Hammond <jhammond@whamcloud.com>
Reviewed-by: Dmitry Eremin <dmitry.eremin@intel.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/ptlrpc/client.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ptlrpc/client.c b/drivers/staging/lustre/lustre/ptlrpc/client.c
index 4bf26a4..eddb920 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/client.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/client.c
@@ -1978,12 +1978,14 @@ int ptlrpc_check_set(const struct lu_env *env, struct ptlrpc_request_set *set)
 		}
 		ptlrpc_rqphase_move(req, RQ_PHASE_COMPLETE);
 
-		CDEBUG(req->rq_reqmsg ? D_RPCTRACE : 0,
-		       "Completed RPC pname:cluuid:pid:xid:nid:opc %s:%s:%d:%llu:%s:%d\n",
-		       current->comm, imp->imp_obd->obd_uuid.uuid,
-		       lustre_msg_get_status(req->rq_reqmsg), req->rq_xid,
-		       libcfs_nid2str(imp->imp_connection->c_peer.nid),
-		       lustre_msg_get_opc(req->rq_reqmsg));
+		if (req->rq_reqmsg)
+			CDEBUG(D_RPCTRACE,
+			       "Completed RPC pname:cluuid:pid:xid:nid:opc %s:%s:%d:%llu:%s:%d\n",
+			       current->comm, imp->imp_obd->obd_uuid.uuid,
+			       lustre_msg_get_status(req->rq_reqmsg),
+			       req->rq_xid,
+			       libcfs_nid2str(imp->imp_connection->c_peer.nid),
+			       lustre_msg_get_opc(req->rq_reqmsg));
 
 		spin_lock(&imp->imp_lock);
 		/*
-- 
1.8.3.1

[lustre-devel] [PATCH 07/30] lustre: llog: update llog print format to use FIDs

[James Simmons]

From: Andreas Dilger <adilger@whamcloud.com>

Print llog identifiers using FIDs instead of OSTID format, which has
been deprecated since 2.3.

Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-9153
Reviewed-on: https://review.whamcloud.com/25640
Reviewed-by: Henri Doreau <henri.doreau@cea.fr>
Reviewed-by: Faccini Bruno <bruno.faccini@intel.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/obdclass/llog_cat.c  | 28 ++++++++++++----------
 drivers/staging/lustre/lustre/obdclass/llog_swab.c |  4 ++--
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/lustre/lustre/obdclass/llog_cat.c b/drivers/staging/lustre/lustre/obdclass/llog_cat.c
index 58dbd50..172a368 100644
--- a/drivers/staging/lustre/lustre/obdclass/llog_cat.c
+++ b/drivers/staging/lustre/lustre/obdclass/llog_cat.c
@@ -79,10 +79,10 @@ static int llog_cat_id2handle(const struct lu_env *env,
 		if (ostid_id(&cgl->lgl_oi) == ostid_id(&logid->lgl_oi) &&
 		    ostid_seq(&cgl->lgl_oi) == ostid_seq(&logid->lgl_oi)) {
 			if (cgl->lgl_ogen != logid->lgl_ogen) {
-				CERROR("%s: log " DOSTID " generation %x != %x\n",
-				       loghandle->lgh_ctxt->loc_obd->obd_name,
-				       POSTID(&logid->lgl_oi), cgl->lgl_ogen,
-				       logid->lgl_ogen);
+				CWARN("%s: log " DFID " generation %x != %x\n",
+				      loghandle->lgh_ctxt->loc_obd->obd_name,
+				      PFID(&logid->lgl_oi.oi_fid),
+				      cgl->lgl_ogen, logid->lgl_ogen);
 				continue;
 			}
 			loghandle->u.phd.phd_cat_handle = cathandle;
@@ -96,9 +96,9 @@ static int llog_cat_id2handle(const struct lu_env *env,
 	rc = llog_open(env, cathandle->lgh_ctxt, &loghandle, logid, NULL,
 		       LLOG_OPEN_EXISTS);
 	if (rc < 0) {
-		CERROR("%s: error opening log id " DOSTID ":%x: rc = %d\n",
+		CERROR("%s: error opening log id " DFID ":%x: rc = %d\n",
 		       cathandle->lgh_ctxt->loc_obd->obd_name,
-		       POSTID(&logid->lgl_oi), logid->lgl_ogen, rc);
+		       PFID(&logid->lgl_oi.oi_fid), logid->lgl_ogen, rc);
 		return rc;
 	}
 
@@ -153,15 +153,16 @@ static int llog_cat_process_cb(const struct lu_env *env,
 		CERROR("invalid record in catalog\n");
 		return -EINVAL;
 	}
-	CDEBUG(D_HA, "processing log " DOSTID ":%x at index %u of catalog "
-	       DOSTID "\n", POSTID(&lir->lid_id.lgl_oi), lir->lid_id.lgl_ogen,
-	       rec->lrh_index, POSTID(&cat_llh->lgh_id.lgl_oi));
+	CDEBUG(D_HA,
+	       "processing log " DFID ":%x at index %u of catalog " DFID "\n",
+	       PFID(&lir->lid_id.lgl_oi.oi_fid), lir->lid_id.lgl_ogen,
+	       rec->lrh_index, PFID(&cat_llh->lgh_id.lgl_oi.oi_fid));
 
 	rc = llog_cat_id2handle(env, cat_llh, &llh, &lir->lid_id);
 	if (rc) {
-		CERROR("%s: cannot find handle for llog " DOSTID ": %d\n",
+		CERROR("%s: cannot find handle for llog " DFID ": %d\n",
 		       cat_llh->lgh_ctxt->loc_obd->obd_name,
-		       POSTID(&lir->lid_id.lgl_oi), rc);
+		       PFID(&lir->lid_id.lgl_oi.oi_fid), rc);
 		return rc;
 	}
 
@@ -206,8 +207,9 @@ static int llog_cat_process_or_fork(const struct lu_env *env,
 	if (llh->llh_cat_idx > cat_llh->lgh_last_idx) {
 		struct llog_process_cat_data cd;
 
-		CWARN("catlog " DOSTID " crosses index zero\n",
-		      POSTID(&cat_llh->lgh_id.lgl_oi));
+		CWARN("%s: catlog " DFID " crosses index zero\n",
+		      cat_llh->lgh_ctxt->loc_obd->obd_name,
+		      PFID(&cat_llh->lgh_id.lgl_oi.oi_fid));
 
 		cd.lpcd_first_idx = llh->llh_cat_idx;
 		cd.lpcd_last_idx = 0;
diff --git a/drivers/staging/lustre/lustre/obdclass/llog_swab.c b/drivers/staging/lustre/lustre/obdclass/llog_swab.c
index 3803056..f18330f 100644
--- a/drivers/staging/lustre/lustre/obdclass/llog_swab.c
+++ b/drivers/staging/lustre/lustre/obdclass/llog_swab.c
@@ -45,8 +45,8 @@
 static void print_llogd_body(struct llogd_body *d)
 {
 	CDEBUG(D_OTHER, "llogd body: %p\n", d);
-	CDEBUG(D_OTHER, "\tlgd_logid.lgl_oi: " DOSTID "\n",
-	       POSTID(&d->lgd_logid.lgl_oi));
+	CDEBUG(D_OTHER, "\tlgd_logid.lgl_oi: " DFID "\n",
+	       PFID(&d->lgd_logid.lgl_oi.oi_fid));
 	CDEBUG(D_OTHER, "\tlgd_logid.lgl_ogen: %#x\n", d->lgd_logid.lgl_ogen);
 	CDEBUG(D_OTHER, "\tlgd_ctxt_idx: %#x\n", d->lgd_ctxt_idx);
 	CDEBUG(D_OTHER, "\tlgd_llh_flags: %#x\n", d->lgd_llh_flags);
-- 
1.8.3.1

[lustre-devel] [PATCH 08/30] lustre: mgc: Remove unnecessary checks for config_log_put()

[James Simmons]

From: Steve Guminski <stephenx.guminski@intel.com>

Make config_log_put() check if its parameter is NULL, which makes
it is unnecessary to perform the check prior to calling it.
This patch removes the redundant checks.

Signed-off-by: Steve Guminski <stephenx.guminski@intel.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-9152
Reviewed-on: https://review.whamcloud.com/25854
Reviewed-by: Bob Glossman <bob.glossman@intel.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: James Simmons <uja.ornl@yahoo.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/mgc/mgc_request.c | 36 +++++++++++--------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
index 4552cc5..e6f8d9e 100644
--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
@@ -131,6 +131,9 @@ static int config_log_get(struct config_llog_data *cld)
  */
 static void config_log_put(struct config_llog_data *cld)
 {
+	if (unlikely(!cld))
+		return;
+
 	CDEBUG(D_INFO, "log %s refs %d\n", cld->cld_logname,
 	       atomic_read(&cld->cld_refcount));
 	LASSERT(atomic_read(&cld->cld_refcount) > 0);
@@ -142,12 +145,9 @@ static void config_log_put(struct config_llog_data *cld)
 
 		CDEBUG(D_MGC, "dropping config log %s\n", cld->cld_logname);
 
-		if (cld->cld_recover)
-			config_log_put(cld->cld_recover);
-		if (cld->cld_params)
-			config_log_put(cld->cld_params);
-		if (cld->cld_sptlrpc)
-			config_log_put(cld->cld_sptlrpc);
+		config_log_put(cld->cld_recover);
+		config_log_put(cld->cld_params);
+		config_log_put(cld->cld_sptlrpc);
 		if (cld_is_sptlrpc(cld))
 			sptlrpc_conf_log_stop(cld->cld_logname);
 
@@ -387,6 +387,9 @@ struct config_llog_data *do_config_log_add(struct obd_device *obd,
 
 static inline void config_mark_cld_stop(struct config_llog_data *cld)
 {
+	if (!cld)
+		return;
+
 	mutex_lock(&cld->cld_lock);
 	spin_lock(&config_list_lock);
 	cld->cld_stopping = 1;
@@ -436,18 +439,13 @@ static int config_log_end(char *logname, struct config_llog_instance *cfg)
 	cld->cld_sptlrpc = NULL;
 	mutex_unlock(&cld->cld_lock);
 
-	if (cld_recover) {
-		config_mark_cld_stop(cld_recover);
-		config_log_put(cld_recover);
-	}
+	config_mark_cld_stop(cld_recover);
+	config_log_put(cld_recover);
 
-	if (cld_params) {
-		config_mark_cld_stop(cld_params);
-		config_log_put(cld_params);
-	}
+	config_mark_cld_stop(cld_params);
+	config_log_put(cld_params);
 
-	if (cld_sptlrpc)
-		config_log_put(cld_sptlrpc);
+	config_log_put(cld_sptlrpc);
 
 	/* drop the ref from the find */
 	config_log_put(cld);
@@ -593,8 +591,7 @@ static int mgc_requeue_thread(void *data)
 			cld->cld_lostlock = 0;
 			spin_unlock(&config_list_lock);
 
-			if (cld_prev)
-				config_log_put(cld_prev);
+			config_log_put(cld_prev);
 			cld_prev = cld;
 
 			if (likely(!(rq_state & RQ_STOP))) {
@@ -606,8 +603,7 @@ static int mgc_requeue_thread(void *data)
 			}
 		}
 		spin_unlock(&config_list_lock);
-		if (cld_prev)
-			config_log_put(cld_prev);
+		config_log_put(cld_prev);
 
 		/* Wait a bit to see if anyone else needs a requeue */
 		wait_event_idle(rq_waitq, rq_state & (RQ_NOW | RQ_STOP));
-- 
1.8.3.1

[lustre-devel] [PATCH 09/30] lustre: obd: remove obsolete OBD_FL_LOCAL_MASK

[James Simmons]

From: Wang Shilong <wshilong@ddn.com>

Andreas Dilger reported that the OBD_FL_LOCAL_MASK support has
not been used since commit e62f0a3c5b9 b=21980 cache ll_obdo_cache:
Can't free all objects; which predates the linux client. With the
future landing of patch https://review.whamcloud.com/26463
lustre: quota: add project inherit attributes the handling of the
"local" flags is broken and since it is unused it is better to be
removed entirely.

Signed-off-by: Wang Shilong <wshilong@ddn.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-9355
Reviewed-on: https://review.whamcloud.com/26728
Reported-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Niu Yawei <yawei.niu@intel.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/include/uapi/linux/lustre/lustre_idl.h |  4 +---
 drivers/staging/lustre/lustre/obdclass/obdo.c                 | 11 -----------
 drivers/staging/lustre/lustre/osc/osc_request.c               |  5 ++---
 drivers/staging/lustre/lustre/ptlrpc/wiretest.c               |  1 -
 4 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/lustre/include/uapi/linux/lustre/lustre_idl.h b/drivers/staging/lustre/include/uapi/linux/lustre/lustre_idl.h
index 58321eb..66b15c7 100644
--- a/drivers/staging/lustre/include/uapi/linux/lustre/lustre_idl.h
+++ b/drivers/staging/lustre/include/uapi/linux/lustre/lustre_idl.h
@@ -931,15 +931,13 @@ enum obdo_flags {
 	OBD_FL_NOSPC_BLK    = 0x00100000, /* no more block space on OST */
 	OBD_FL_FLUSH        = 0x00200000, /* flush pages on the OST */
 	OBD_FL_SHORT_IO     = 0x00400000, /* short io request */
+	/* OBD_FL_LOCAL_MASK = 0xF0000000, was local-only flags until 2.10 */
 
 	/* Note that while these checksum values are currently separate bits,
 	 * in 2.x we can actually allow all values from 1-31 if we wanted.
 	 */
 	OBD_FL_CKSUM_ALL    = OBD_FL_CKSUM_CRC32 | OBD_FL_CKSUM_ADLER |
 			      OBD_FL_CKSUM_CRC32C,
-
-	/* mask for local-only flag, which won't be sent over network */
-	OBD_FL_LOCAL_MASK   = 0xF0000000,
 };
 
 /*
diff --git a/drivers/staging/lustre/lustre/obdclass/obdo.c b/drivers/staging/lustre/lustre/obdclass/obdo.c
index 5b2bfa8..8013c1e 100644
--- a/drivers/staging/lustre/lustre/obdclass/obdo.c
+++ b/drivers/staging/lustre/lustre/obdclass/obdo.c
@@ -134,7 +134,6 @@ void lustre_set_wire_obdo(const struct obd_connect_data *ocd,
 			  struct obdo *wobdo, const struct obdo *lobdo)
 {
 	*wobdo = *lobdo;
-	wobdo->o_flags &= ~OBD_FL_LOCAL_MASK;
 	if (!ocd)
 		return;
 
@@ -156,17 +155,7 @@ void lustre_set_wire_obdo(const struct obd_connect_data *ocd,
 void lustre_get_wire_obdo(const struct obd_connect_data *ocd,
 			  struct obdo *lobdo, const struct obdo *wobdo)
 {
-	u32 local_flags = 0;
-
-	if (lobdo->o_valid & OBD_MD_FLFLAGS)
-		local_flags = lobdo->o_flags & OBD_FL_LOCAL_MASK;
-
 	*lobdo = *wobdo;
-	if (local_flags) {
-		lobdo->o_valid |= OBD_MD_FLFLAGS;
-		lobdo->o_flags &= ~OBD_FL_LOCAL_MASK;
-		lobdo->o_flags |= local_flags;
-	}
 	if (!ocd)
 		return;
 
diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c b/drivers/staging/lustre/lustre/osc/osc_request.c
index 8024b17..5bab8a3 100644
--- a/drivers/staging/lustre/lustre/osc/osc_request.c
+++ b/drivers/staging/lustre/lustre/osc/osc_request.c
@@ -1273,10 +1273,9 @@ static int osc_brw_prep_request(int cmd, struct client_obd *cli,
 			 */
 			enum cksum_type cksum_type = cli->cl_cksum_type;
 
-			if ((body->oa.o_valid & OBD_MD_FLFLAGS) == 0) {
-				oa->o_flags &= OBD_FL_LOCAL_MASK;
+			if ((body->oa.o_valid & OBD_MD_FLFLAGS) == 0)
 				body->oa.o_flags = 0;
-			}
+
 			body->oa.o_flags |= cksum_type_pack(cksum_type);
 			body->oa.o_valid |= OBD_MD_FLCKSUM | OBD_MD_FLFLAGS;
 			body->oa.o_cksum = osc_checksum_bulk(requested_nob,
diff --git a/drivers/staging/lustre/lustre/ptlrpc/wiretest.c b/drivers/staging/lustre/lustre/ptlrpc/wiretest.c
index 09b1298..09aef9a 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/wiretest.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/wiretest.c
@@ -1344,7 +1344,6 @@ void lustre_assert_wire_constants(void)
 	BUILD_BUG_ON(OBD_FL_MMAP != 0x00040000);
 	BUILD_BUG_ON(OBD_FL_RECOV_RESEND != 0x00080000);
 	BUILD_BUG_ON(OBD_FL_NOSPC_BLK != 0x00100000);
-	BUILD_BUG_ON(OBD_FL_LOCAL_MASK != 0xf0000000);
 
 	/* Checks for struct lov_ost_data_v1 */
 	LASSERTF((int)sizeof(struct lov_ost_data_v1) == 24, "found %lld\n",
-- 
1.8.3.1

[lustre-devel] [PATCH 10/30] lustre: ptlrpc: set rq_sent when send fails due to -ENOMEM

[James Simmons]

From: "John L. Hammond" <jhammond@whamcloud.com>

In ptl_send_rpc() set rq_sent when we fail to send the RPC due
to insufficient memory, since this is what the upper layers expect.

Signed-off-by: John L. Hammond <jhammond@whamcloud.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-9296
Reviewed-on: https://review.whamcloud.com/26470
Reviewed-by: Jinshan Xiong <jinshan.xiong@gmail.com>
Reviewed-by: Mike Pershin <mpershin@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/ptlrpc/niobuf.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ptlrpc/niobuf.c b/drivers/staging/lustre/lustre/ptlrpc/niobuf.c
index 0e13fb9..3cfd7b8 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/niobuf.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/niobuf.c
@@ -571,15 +571,8 @@ int ptl_send_rpc(struct ptlrpc_request *request, int noreply)
 		mpflag = memalloc_noreclaim_save();
 
 	rc = sptlrpc_cli_wrap_request(request);
-	if (rc) {
-		/*
-		 * set rq_sent so that this request is treated
-		 * as a delayed send in the upper layers
-		 */
-		if (rc == -ENOMEM)
-			request->rq_sent = ktime_get_seconds();
+	if (rc)
 		goto out;
-	}
 
 	/* bulk register should be done after wrap_request() */
 	if (request->rq_bulk) {
@@ -724,9 +717,18 @@ int ptl_send_rpc(struct ptlrpc_request *request, int noreply)
 	 * the chance to have long unlink to sluggish net is smaller here.
 	 */
 	ptlrpc_unregister_bulk(request, 0);
- out:
+out:
+	if (rc == -ENOMEM) {
+		/*
+		 * set rq_sent so that this request is treated
+		 * as a delayed send in the upper layers
+		 */
+		request->rq_sent = ktime_get_seconds();
+	}
+
 	if (request->rq_memalloc)
 		memalloc_noreclaim_restore(mpflag);
+
 	return rc;
 }
 EXPORT_SYMBOL(ptl_send_rpc);
-- 
1.8.3.1

[lustre-devel] [PATCH 11/30] lustre: mdc: allow setting readdir RPC size parameter

[James Simmons]

From: Andreas Dilger <adilger@whamcloud.com>

Allow the mdc.*.max_pages_per_rpc tunable to set the MDS bulk
readdir RPC size, rather than always using the default 1MB RPC
size. The tunable is set in the MDC, as it should be, rather
than in the llite superblock, which requires extra code just to
get it up from the MDC's connect_data only to send it back down.
The RPC size could be tuned independently if different types of
MDSes are used (e.g. local vs. remote).

Remove the md_op_data.op_max_pages and ll_sb_info.ll_md_brw_pages
fields that previously were used to pass the readdir size from
llite to mdc_read_page(). Reorder some 32-bit fields in md_op_data
to avoid struct holes.

Move osc's max_pages_per_rpc_store() to obdclass along with
max_pages_per_rpc_show().

Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-3308
Reviewed-on: https://review.whamcloud.com/26088
Reviewed-by: Niu Yawei <yawei.niu@intel.com>
Reviewed-by: Lai Siyao <lai.siyao@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 .../staging/lustre/lustre/include/lprocfs_status.h |  5 ++
 drivers/staging/lustre/lustre/include/lustre_net.h |  8 ++--
 drivers/staging/lustre/lustre/include/obd.h        |  5 +-
 drivers/staging/lustre/lustre/llite/dir.c          | 12 +----
 drivers/staging/lustre/lustre/llite/lcommon_cl.c   | 19 +++-----
 .../staging/lustre/lustre/llite/llite_internal.h   |  2 -
 drivers/staging/lustre/lustre/llite/llite_lib.c    |  5 --
 drivers/staging/lustre/lustre/llite/llite_nfs.c    |  1 -
 drivers/staging/lustre/lustre/llite/statahead.c    |  4 --
 drivers/staging/lustre/lustre/mdc/lproc_mdc.c      | 20 +-------
 drivers/staging/lustre/lustre/mdc/mdc_request.c    | 13 +++---
 .../lustre/lustre/obdclass/lprocfs_status.c        | 54 ++++++++++++++++++++++
 drivers/staging/lustre/lustre/osc/lproc_osc.c      | 46 ------------------
 drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c    |  2 +-
 14 files changed, 82 insertions(+), 114 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lprocfs_status.h b/drivers/staging/lustre/lustre/include/lprocfs_status.h
index c841aba..5da26e3 100644
--- a/drivers/staging/lustre/lustre/include/lprocfs_status.h
+++ b/drivers/staging/lustre/lustre/include/lprocfs_status.h
@@ -584,6 +584,11 @@ ssize_t lustre_attr_store(struct kobject *kobj, struct attribute *attr,
 
 extern const struct sysfs_ops lustre_sysfs_ops;
 
+ssize_t max_pages_per_rpc_show(struct kobject *kobj, struct attribute *attr,
+			       char *buf);
+ssize_t max_pages_per_rpc_store(struct kobject *kobj, struct attribute *attr,
+				const char *buffer, size_t count);
+
 struct root_squash_info;
 int lprocfs_wr_root_squash(const char __user *buffer, unsigned long count,
 			   struct root_squash_info *squash, char *name);
diff --git a/drivers/staging/lustre/lustre/include/lustre_net.h b/drivers/staging/lustre/lustre/include/lustre_net.h
index 2dbd208..cf630db 100644
--- a/drivers/staging/lustre/lustre/include/lustre_net.h
+++ b/drivers/staging/lustre/lustre/include/lustre_net.h
@@ -104,15 +104,15 @@
  * currently supported maximum between peers at connect via ocd_brw_size.
  */
 #define PTLRPC_MAX_BRW_BITS	(LNET_MTU_BITS + PTLRPC_BULK_OPS_BITS)
-#define PTLRPC_MAX_BRW_SIZE	(1 << PTLRPC_MAX_BRW_BITS)
+#define PTLRPC_MAX_BRW_SIZE	BIT(PTLRPC_MAX_BRW_BITS)
 #define PTLRPC_MAX_BRW_PAGES	(PTLRPC_MAX_BRW_SIZE >> PAGE_SHIFT)
 
-#define ONE_MB_BRW_SIZE		(1 << LNET_MTU_BITS)
-#define MD_MAX_BRW_SIZE		(1 << LNET_MTU_BITS)
+#define ONE_MB_BRW_SIZE		BIT(LNET_MTU_BITS)
+#define MD_MAX_BRW_SIZE		BIT(LNET_MTU_BITS)
 #define MD_MAX_BRW_PAGES	(MD_MAX_BRW_SIZE >> PAGE_SHIFT)
 #define DT_MAX_BRW_SIZE		PTLRPC_MAX_BRW_SIZE
 #define DT_MAX_BRW_PAGES	(DT_MAX_BRW_SIZE >> PAGE_SHIFT)
-#define OFD_MAX_BRW_SIZE	(1 << LNET_MTU_BITS)
+#define OFD_MAX_BRW_SIZE	BIT(LNET_MTU_BITS)
 
 /* When PAGE_SIZE is a constant, we can check our arithmetic here with cpp! */
 # if ((PTLRPC_MAX_BRW_PAGES & (PTLRPC_MAX_BRW_PAGES - 1)) != 0)
diff --git a/drivers/staging/lustre/lustre/include/obd.h b/drivers/staging/lustre/lustre/include/obd.h
index 329bae9..50e97b4 100644
--- a/drivers/staging/lustre/lustre/include/obd.h
+++ b/drivers/staging/lustre/lustre/include/obd.h
@@ -717,11 +717,11 @@ struct md_op_data {
 	struct lu_fid	   op_fid3; /* 2 extra fids to find conflicting */
 	struct lu_fid	   op_fid4; /* to the operation locks. */
 	u32			op_mds;  /* what mds server open will go to */
+	u32			op_mode;
 	struct lustre_handle    op_handle;
 	s64			op_mod_time;
 	const char	     *op_name;
 	size_t			op_namelen;
-	__u32		   op_mode;
 	struct lmv_stripe_md   *op_mea1;
 	struct lmv_stripe_md   *op_mea2;
 	__u32		   op_suppgids[2];
@@ -746,9 +746,6 @@ struct md_op_data {
 	/* Used by readdir */
 	__u64		   op_offset;
 
-	/* Used by readdir */
-	__u32			op_max_pages;
-
 	/* used to transfer info between the stacks of MD client
 	 * see enum op_cli_flags
 	 */
diff --git a/drivers/staging/lustre/lustre/llite/dir.c b/drivers/staging/lustre/lustre/llite/dir.c
index f352fab..26b0c2d 100644
--- a/drivers/staging/lustre/lustre/llite/dir.c
+++ b/drivers/staging/lustre/lustre/llite/dir.c
@@ -231,18 +231,11 @@ int ll_dir_read(struct inode *inode, __u64 *ppos, struct md_op_data *op_data,
 			__u64	  ino;
 
 			hash = le64_to_cpu(ent->lde_hash);
-			if (hash < pos)
-				/*
-				 * Skip until we find target hash
-				 * value.
-				 */
+			if (hash < pos) /* Skip until we find target hash */
 				continue;
 
 			namelen = le16_to_cpu(ent->lde_namelen);
-			if (namelen == 0)
-				/*
-				 * Skip dummy record.
-				 */
+			if (namelen == 0) /* Skip dummy record. */
 				continue;
 
 			if (is_api32 && is_hash64)
@@ -351,7 +344,6 @@ static int ll_readdir(struct file *filp, struct dir_context *ctx)
 			}
 		}
 	}
-	op_data->op_max_pages = sbi->ll_md_brw_pages;
 	ctx->pos = pos;
 	rc = ll_dir_read(inode, &pos, op_data, ctx);
 	pos = ctx->pos;
diff --git a/drivers/staging/lustre/lustre/llite/lcommon_cl.c b/drivers/staging/lustre/lustre/llite/lcommon_cl.c
index 6c9fe49..d3b0445 100644
--- a/drivers/staging/lustre/lustre/llite/lcommon_cl.c
+++ b/drivers/staging/lustre/lustre/llite/lcommon_cl.c
@@ -267,27 +267,22 @@ void cl_inode_fini(struct inode *inode)
 /**
  * build inode number from passed @fid
  */
-__u64 cl_fid_build_ino(const struct lu_fid *fid, int api32)
+u64 cl_fid_build_ino(const struct lu_fid *fid, int api32)
 {
 	if (BITS_PER_LONG == 32 || api32)
 		return fid_flatten32(fid);
-	else
-		return fid_flatten(fid);
+
+	return fid_flatten(fid);
 }
 
 /**
  * build inode generation from passed @fid.  If our FID overflows the 32-bit
  * inode number then return a non-zero generation to distinguish them.
  */
-__u32 cl_fid_build_gen(const struct lu_fid *fid)
+u32 cl_fid_build_gen(const struct lu_fid *fid)
 {
-	__u32 gen;
-
-	if (fid_is_igif(fid)) {
-		gen = lu_igif_gen(fid);
-		return gen;
-	}
+	if (fid_is_igif(fid))
+		return lu_igif_gen(fid);
 
-	gen = fid_flatten(fid) >> 32;
-	return gen;
+	return fid_flatten(fid) >> 32;
 }
diff --git a/drivers/staging/lustre/lustre/llite/llite_internal.h b/drivers/staging/lustre/lustre/llite/llite_internal.h
index bf679c9..9d9f623 100644
--- a/drivers/staging/lustre/lustre/llite/llite_internal.h
+++ b/drivers/staging/lustre/lustre/llite/llite_internal.h
@@ -490,8 +490,6 @@ struct ll_sb_info {
 	unsigned int	      ll_namelen;
 	const struct file_operations	*ll_fop;
 
-	unsigned int		  ll_md_brw_pages; /* readdir pages per RPC */
-
 	struct lu_site	   *ll_site;
 	struct cl_device	 *ll_cl;
 	/* Statistics */
diff --git a/drivers/staging/lustre/lustre/llite/llite_lib.c b/drivers/staging/lustre/lustre/llite/llite_lib.c
index f0fe21f..b5bafc4 100644
--- a/drivers/staging/lustre/lustre/llite/llite_lib.c
+++ b/drivers/staging/lustre/lustre/llite/llite_lib.c
@@ -343,11 +343,6 @@ static int client_common_fill_super(struct super_block *sb, char *md, char *dt)
 	if (data->ocd_connect_flags & OBD_CONNECT_64BITHASH)
 		sbi->ll_flags |= LL_SBI_64BIT_HASH;
 
-	if (data->ocd_connect_flags & OBD_CONNECT_BRW_SIZE)
-		sbi->ll_md_brw_pages = data->ocd_brw_size >> PAGE_SHIFT;
-	else
-		sbi->ll_md_brw_pages = 1;
-
 	if (data->ocd_connect_flags & OBD_CONNECT_LAYOUTLOCK)
 		sbi->ll_flags |= LL_SBI_LAYOUT_LOCK;
 
diff --git a/drivers/staging/lustre/lustre/llite/llite_nfs.c b/drivers/staging/lustre/lustre/llite/llite_nfs.c
index 9efb20e..c66072a 100644
--- a/drivers/staging/lustre/lustre/llite/llite_nfs.c
+++ b/drivers/staging/lustre/lustre/llite/llite_nfs.c
@@ -261,7 +261,6 @@ static int ll_get_name(struct dentry *dentry, char *name,
 		goto out;
 	}
 
-	op_data->op_max_pages = ll_i2sbi(dir)->ll_md_brw_pages;
 	inode_lock(dir);
 	rc = ll_dir_read(dir, &pos, op_data, &lgd.ctx);
 	inode_unlock(dir);
diff --git a/drivers/staging/lustre/lustre/llite/statahead.c b/drivers/staging/lustre/lustre/llite/statahead.c
index ea2a002..1ad308c 100644
--- a/drivers/staging/lustre/lustre/llite/statahead.c
+++ b/drivers/staging/lustre/lustre/llite/statahead.c
@@ -961,8 +961,6 @@ static int ll_statahead_thread(void *arg)
 		goto out;
 	}
 
-	op_data->op_max_pages = ll_i2sbi(dir)->ll_md_brw_pages;
-
 	while (pos != MDS_DIR_END_OFF && sai->sai_task) {
 		struct lu_dirpage *dp;
 		struct lu_dirent  *ent;
@@ -1215,8 +1213,6 @@ static int is_first_dirent(struct inode *dir, struct dentry *dentry)
 	/**
 	 * FIXME choose the start offset of the readdir
 	 */
-	op_data->op_max_pages = ll_i2sbi(dir)->ll_md_brw_pages;
-
 	page = ll_get_dir_page(dir, op_data, pos);
 
 	while (1) {
diff --git a/drivers/staging/lustre/lustre/mdc/lproc_mdc.c b/drivers/staging/lustre/lustre/mdc/lproc_mdc.c
index 3bff8b5..a205c61 100644
--- a/drivers/staging/lustre/lustre/mdc/lproc_mdc.c
+++ b/drivers/staging/lustre/lustre/mdc/lproc_mdc.c
@@ -134,6 +134,8 @@ static ssize_t max_mod_rpcs_in_flight_store(struct kobject *kobj,
 }
 LUSTRE_RW_ATTR(max_mod_rpcs_in_flight);
 
+LUSTRE_RW_ATTR(max_pages_per_rpc);
+
 #define mdc_conn_uuid_show conn_uuid_show
 LUSTRE_RO_ATTR(mdc_conn_uuid);
 
@@ -165,24 +167,6 @@ static ssize_t mdc_rpc_stats_seq_write(struct file *file,
 LPROC_SEQ_FOPS_RO_TYPE(mdc, timeouts);
 LPROC_SEQ_FOPS_RO_TYPE(mdc, state);
 
-/*
- * Note: below sysfs entry is provided, but not currently in use, instead
- * sbi->sb_md_brw_size is used, the per obd variable should be used
- * when DNE is enabled, and dir pages are managed in MDC layer.
- * Don't forget to enable sysfs store function then.
- */
-static ssize_t max_pages_per_rpc_show(struct kobject *kobj,
-				      struct attribute *attr,
-				      char *buf)
-{
-	struct obd_device *dev = container_of(kobj, struct obd_device,
-					      obd_kset.kobj);
-	struct client_obd *cli = &dev->u.cli;
-
-	return sprintf(buf, "%d\n", cli->cl_max_pages_per_rpc);
-}
-LUSTRE_RO_ATTR(max_pages_per_rpc);
-
 LPROC_SEQ_FOPS_RW_TYPE(mdc, import);
 LPROC_SEQ_FOPS_RW_TYPE(mdc, pinger_recov);
 
diff --git a/drivers/staging/lustre/lustre/mdc/mdc_request.c b/drivers/staging/lustre/lustre/mdc/mdc_request.c
index 116e973..37bf486 100644
--- a/drivers/staging/lustre/lustre/mdc/mdc_request.c
+++ b/drivers/staging/lustre/lustre/mdc/mdc_request.c
@@ -1166,17 +1166,17 @@ static int mdc_read_page_remote(void *data, struct page *page0)
 	struct page **page_pool;
 	struct page *page;
 	struct lu_dirpage *dp;
-	int rd_pgs = 0; /* number of pages read actually */
-	int npages;
 	struct md_op_data *op_data = rp->rp_mod;
 	struct ptlrpc_request *req;
-	int max_pages = op_data->op_max_pages;
 	struct inode *inode;
 	struct lu_fid *fid;
-	int i;
+	int rd_pgs = 0; /* number of pages read actually */
+	int max_pages;
+	int npages;
 	int rc;
+	int i;
 
-	LASSERT(max_pages > 0 && max_pages <= PTLRPC_MAX_BRW_PAGES);
+	max_pages = rp->rp_exp->exp_obd->u.cli.cl_max_pages_per_rpc;
 	inode = op_data->op_data;
 	fid = &op_data->op_fid1;
 	LASSERT(inode);
@@ -1200,8 +1200,7 @@ static int mdc_read_page_remote(void *data, struct page *page0)
 	if (!rc) {
 		int lu_pgs = req->rq_bulk->bd_nob_transferred;
 
-		rd_pgs = (req->rq_bulk->bd_nob_transferred +
-			  PAGE_SIZE - 1) >> PAGE_SHIFT;
+		rd_pgs = (lu_pgs + PAGE_SIZE - 1) >> PAGE_SHIFT;
 		lu_pgs >>= LU_PAGE_SHIFT;
 		LASSERT(!(req->rq_bulk->bd_nob_transferred & ~LU_PAGE_MASK));
 
diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
index 84e5a8c..a540abb 100644
--- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
+++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
@@ -1785,3 +1785,57 @@ ssize_t lustre_attr_store(struct kobject *kobj, struct attribute *attr,
 	.store = lustre_attr_store,
 };
 EXPORT_SYMBOL_GPL(lustre_sysfs_ops);
+
+ssize_t max_pages_per_rpc_show(struct kobject *kobj, struct attribute *attr,
+			       char *buf)
+{
+	struct obd_device *dev = container_of(kobj, struct obd_device,
+					      obd_kset.kobj);
+	struct client_obd *cli = &dev->u.cli;
+
+	return sprintf(buf, "%d\n", cli->cl_max_pages_per_rpc);
+}
+EXPORT_SYMBOL(max_pages_per_rpc_show);
+
+ssize_t max_pages_per_rpc_store(struct kobject *kobj, struct attribute *attr,
+				const char *buffer, size_t count)
+{
+	struct obd_device *dev = container_of(kobj, struct obd_device,
+					      obd_kset.kobj);
+	struct client_obd *cli = &dev->u.cli;
+	struct obd_connect_data *ocd;
+	unsigned long long val;
+	int chunk_mask;
+	int rc;
+
+	rc = kstrtoull(buffer, 10, &val);
+	if (rc)
+		return rc;
+
+	/* if the max_pages is specified in bytes, convert to pages */
+	if (val >= ONE_MB_BRW_SIZE)
+		val >>= PAGE_SHIFT;
+
+	rc = lprocfs_climp_check(dev);
+	if (rc)
+		return rc;
+
+	ocd = &cli->cl_import->imp_connect_data;
+	chunk_mask = ~((1 << (cli->cl_chunkbits - PAGE_SHIFT)) - 1);
+	/* max_pages_per_rpc must be chunk aligned */
+	val = (val + ~chunk_mask) & chunk_mask;
+	if (!val || (ocd->ocd_brw_size &&
+		     val > ocd->ocd_brw_size >> PAGE_SHIFT)) {
+		up_read(&dev->u.cli.cl_sem);
+		return -ERANGE;
+	}
+
+	spin_lock(&cli->cl_loi_list_lock);
+	cli->cl_max_pages_per_rpc = val;
+	client_adjust_max_dirty(cli);
+	spin_unlock(&cli->cl_loi_list_lock);
+
+	up_read(&dev->u.cli.cl_sem);
+	return count;
+}
+EXPORT_SYMBOL(max_pages_per_rpc_store);
diff --git a/drivers/staging/lustre/lustre/osc/lproc_osc.c b/drivers/staging/lustre/lustre/osc/lproc_osc.c
index cd28295..bf576f1 100644
--- a/drivers/staging/lustre/lustre/osc/lproc_osc.c
+++ b/drivers/staging/lustre/lustre/osc/lproc_osc.c
@@ -570,52 +570,6 @@ static ssize_t destroys_in_flight_show(struct kobject *kobj,
 		       atomic_read(&obd->u.cli.cl_destroy_in_flight));
 }
 LUSTRE_RO_ATTR(destroys_in_flight);
-
-static ssize_t max_pages_per_rpc_show(struct kobject *kobj,
-				      struct attribute *attr,
-				      char *buf)
-{
-	struct obd_device *dev = container_of(kobj, struct obd_device,
-					      obd_kset.kobj);
-	struct client_obd *cli = &dev->u.cli;
-
-	return sprintf(buf, "%d\n", cli->cl_max_pages_per_rpc);
-}
-
-static ssize_t max_pages_per_rpc_store(struct kobject *kobj,
-				       struct attribute *attr,
-				       const char *buffer,
-				       size_t count)
-{
-	struct obd_device *dev = container_of(kobj, struct obd_device,
-					      obd_kset.kobj);
-	struct client_obd *cli = &dev->u.cli;
-	struct obd_connect_data *ocd = &cli->cl_import->imp_connect_data;
-	int chunk_mask, rc;
-	unsigned long long val;
-
-	rc = kstrtoull(buffer, 10, &val);
-	if (rc)
-		return rc;
-
-	/* if the max_pages is specified in bytes, convert to pages */
-	if (val >= ONE_MB_BRW_SIZE)
-		val >>= PAGE_SHIFT;
-
-	chunk_mask = ~((1 << (cli->cl_chunkbits - PAGE_SHIFT)) - 1);
-	/* max_pages_per_rpc must be chunk aligned */
-	val = (val + ~chunk_mask) & chunk_mask;
-	if (!val || (ocd->ocd_brw_size &&
-		     val > ocd->ocd_brw_size >> PAGE_SHIFT)) {
-		return -ERANGE;
-	}
-	spin_lock(&cli->cl_loi_list_lock);
-	cli->cl_max_pages_per_rpc = val;
-	client_adjust_max_dirty(cli);
-	spin_unlock(&cli->cl_loi_list_lock);
-
-	return count;
-}
 LUSTRE_RW_ATTR(max_pages_per_rpc);
 
 static int osc_unstable_stats_seq_show(struct seq_file *m, void *v)
diff --git a/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c b/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c
index 625b952..3d336d9 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c
@@ -232,7 +232,7 @@ static unsigned long enc_pools_shrink_count(struct shrinker *s,
 	}
 
 	LASSERT(page_pools.epp_idle_idx <= IDLE_IDX_MAX);
-	return max((int)page_pools.epp_free_pages - PTLRPC_MAX_BRW_PAGES, 0) *
+	return max(page_pools.epp_free_pages - PTLRPC_MAX_BRW_PAGES, 0UL) *
 		(IDLE_IDX_MAX - page_pools.epp_idle_idx) / IDLE_IDX_MAX;
 }
 
-- 
1.8.3.1

[lustre-devel] [PATCH 12/30] lustre: kuc: initialize kkuc_groups at module init time

[James Simmons]

From: "John L. Hammond" <jhammond@whamcloud.com>

Some kkuc functions use kkuc_groups[group].next == NULL to test for an
empty group list. This is incorrect if the group was previously added
to but not empty. Remove all next == NULL tests and use module load
time initialization of the kkuc_groups array.

Signed-off-by: John L. Hammond <jhammond@whamcloud.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-9306
Reviewed-on: https://review.whamcloud.com/26883
Reviewed-by: Frank Zago <fzago@cray.com>
Reviewed-by: Faccini Bruno <bruno.faccini@intel.com>
Reviewed-by: Henri Doreau <henri.doreau@cea.fr>
Reviewed-by: Quentin Bouget <quentin.bouget@cea.fr>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 .../lustre/lustre/include/lustre_kernelcomm.h      |  1 +
 drivers/staging/lustre/lustre/obdclass/class_obd.c |  3 ++
 .../staging/lustre/lustre/obdclass/kernelcomm.c    | 38 +++++++++++++++-------
 3 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_kernelcomm.h b/drivers/staging/lustre/lustre/include/lustre_kernelcomm.h
index 2b3fa84..8e3a990 100644
--- a/drivers/staging/lustre/lustre/include/lustre_kernelcomm.h
+++ b/drivers/staging/lustre/lustre/include/lustre_kernelcomm.h
@@ -45,6 +45,7 @@
 typedef int (*libcfs_kkuc_cb_t)(void *data, void *cb_arg);
 
 /* Kernel methods */
+void libcfs_kkuc_init(void);
 int libcfs_kkuc_msg_put(struct file *fp, void *payload);
 int libcfs_kkuc_group_put(unsigned int group, void *payload);
 int libcfs_kkuc_group_add(struct file *fp, int uid, unsigned int group,
diff --git a/drivers/staging/lustre/lustre/obdclass/class_obd.c b/drivers/staging/lustre/lustre/obdclass/class_obd.c
index eabaafe..2d23608 100644
--- a/drivers/staging/lustre/lustre/obdclass/class_obd.c
+++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c
@@ -42,6 +42,7 @@
 #include <obd_class.h>
 #include <uapi/linux/lnet/lnetctl.h>
 #include <lustre_debug.h>
+#include <lustre_kernelcomm.h>
 #include <lprocfs_status.h>
 #include <linux/list.h>
 #include <cl_object.h>
@@ -664,6 +665,8 @@ static int __init obdclass_init(void)
 	if (err)
 		return err;
 
+	libcfs_kkuc_init();
+
 	err = obd_zombie_impexp_init();
 	if (err)
 		return err;
diff --git a/drivers/staging/lustre/lustre/obdclass/kernelcomm.c b/drivers/staging/lustre/lustre/obdclass/kernelcomm.c
index 304288d..0fcfecf 100644
--- a/drivers/staging/lustre/lustre/obdclass/kernelcomm.c
+++ b/drivers/staging/lustre/lustre/obdclass/kernelcomm.c
@@ -96,10 +96,23 @@ struct kkuc_reg {
 	char		 kr_data[0];
 };
 
-static struct list_head kkuc_groups[KUC_GRP_MAX + 1] = {};
+static struct list_head kkuc_groups[KUC_GRP_MAX + 1];
 /* Protect message sending against remove and adds */
 static DECLARE_RWSEM(kg_sem);
 
+static inline bool libcfs_kkuc_group_is_valid(int group)
+{
+	return 0 <= group && group < ARRAY_SIZE(kkuc_groups);
+}
+
+void libcfs_kkuc_init(void)
+{
+	int group;
+
+	for (group = 0; group < ARRAY_SIZE(kkuc_groups); group++)
+		INIT_LIST_HEAD(&kkuc_groups[group]);
+}
+
 /** Add a receiver to a broadcast group
  * @param filp pipe to write into
  * @param uid identifier for this receiver
@@ -111,7 +124,7 @@ int libcfs_kkuc_group_add(struct file *filp, int uid, unsigned int group,
 {
 	struct kkuc_reg *reg;
 
-	if (group > KUC_GRP_MAX) {
+	if (!libcfs_kkuc_group_is_valid(group)) {
 		CDEBUG(D_WARNING, "Kernelcomm: bad group %d\n", group);
 		return -EINVAL;
 	}
@@ -130,8 +143,6 @@ int libcfs_kkuc_group_add(struct file *filp, int uid, unsigned int group,
 	memcpy(reg->kr_data, data, data_len);
 
 	down_write(&kg_sem);
-	if (!kkuc_groups[group].next)
-		INIT_LIST_HEAD(&kkuc_groups[group]);
 	list_add(&reg->kr_chain, &kkuc_groups[group]);
 	up_write(&kg_sem);
 
@@ -145,8 +156,10 @@ int libcfs_kkuc_group_rem(int uid, unsigned int group)
 {
 	struct kkuc_reg *reg, *next;
 
-	if (!kkuc_groups[group].next)
-		return 0;
+	if (!libcfs_kkuc_group_is_valid(group)) {
+		CDEBUG(D_WARNING, "Kernelcomm: bad group %d\n", group);
+		return -EINVAL;
+	}
 
 	if (!uid) {
 		/* Broadcast a shutdown message */
@@ -182,9 +195,14 @@ int libcfs_kkuc_group_put(unsigned int group, void *payload)
 	int rc = 0;
 	int one_success = 0;
 
+	if (!libcfs_kkuc_group_is_valid(group)) {
+		CDEBUG(D_WARNING, "Kernelcomm: bad group %d\n", group);
+		return -EINVAL;
+	}
+
 	down_write(&kg_sem);
 
-	if (unlikely(!kkuc_groups[group].next) ||
+	if (unlikely(list_empty(&kkuc_groups[group])) ||
 	    unlikely(OBD_FAIL_CHECK(OBD_FAIL_MDS_HSM_CT_REGISTER_NET))) {
 		/* no agent have fully registered, CDT will retry */
 		up_write(&kg_sem);
@@ -227,15 +245,11 @@ int libcfs_kkuc_group_foreach(unsigned int group, libcfs_kkuc_cb_t cb_func,
 	struct kkuc_reg *reg;
 	int rc = 0;
 
-	if (group > KUC_GRP_MAX) {
+	if (!libcfs_kkuc_group_is_valid(group)) {
 		CDEBUG(D_WARNING, "Kernelcomm: bad group %d\n", group);
 		return -EINVAL;
 	}
 
-	/* no link for this group */
-	if (!kkuc_groups[group].next)
-		return 0;
-
 	down_read(&kg_sem);
 	list_for_each_entry(reg, &kkuc_groups[group], kr_chain) {
 		if (reg->kr_fp)
-- 
1.8.3.1

[lustre-devel] [PATCH 13/30] lustre: ldlm: GPF in _ldlm_lock_debug()

[James Simmons]

From: Andriy Skulysh <c17819@cray.com>

Lock's resource can change on a client. Take a resource reference
under spinlock to print debug information.

Signed-off-by: Andriy Skulysh <c17819@cray.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-7062
Xyratex-bug-id: MRP-2760
Reviewed-on: https://review.whamcloud.com/16139
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Alex Zhuravlev <bzzz@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/include/lustre_dlm.h |  1 +
 drivers/staging/lustre/lustre/ldlm/ldlm_lock.c     | 11 +++++++++--
 drivers/staging/lustre/lustre/ldlm/ldlm_resource.c |  2 +-
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_dlm.h b/drivers/staging/lustre/lustre/include/lustre_dlm.h
index 2a05ab8..a68c7a4 100644
--- a/drivers/staging/lustre/lustre/include/lustre_dlm.h
+++ b/drivers/staging/lustre/lustre/include/lustre_dlm.h
@@ -1186,6 +1186,7 @@ struct ldlm_resource *ldlm_resource_get(struct ldlm_namespace *ns,
 					struct ldlm_resource *parent,
 					const struct ldlm_res_id *,
 					enum ldlm_type type, int create);
+struct ldlm_resource *ldlm_resource_getref(struct ldlm_resource *res);
 void ldlm_resource_putref(struct ldlm_resource *res);
 void ldlm_resource_add_lock(struct ldlm_resource *res,
 			    struct list_head *head,
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
index 2fb2e08..3cbdc81 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
@@ -1981,10 +1981,16 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
 		      struct libcfs_debug_msg_data *msgdata,
 		      const char *fmt, ...)
 {
-	va_list args;
 	struct obd_export *exp = lock->l_export;
-	struct ldlm_resource *resource = lock->l_resource;
+	struct ldlm_resource *resource = NULL;
 	char *nid = "local";
+	va_list args;
+
+	if (spin_trylock(&lock->l_lock)) {
+		if (lock->l_resource)
+			resource = ldlm_resource_getref(lock->l_resource);
+		spin_unlock(&lock->l_lock);
+	}
 
 	va_start(args, fmt);
 
@@ -2099,5 +2105,6 @@ void _ldlm_lock_debug(struct ldlm_lock *lock,
 		break;
 	}
 	va_end(args);
+	ldlm_resource_putref(resource);
 }
 EXPORT_SYMBOL(_ldlm_lock_debug);
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c b/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c
index 33d73fa..1907a5a 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c
@@ -402,7 +402,7 @@ static int ldlm_namespace_debugfs_register(struct ldlm_namespace *ns)
 
 #undef MAX_STRING_SIZE
 
-static struct ldlm_resource *ldlm_resource_getref(struct ldlm_resource *res)
+struct ldlm_resource *ldlm_resource_getref(struct ldlm_resource *res)
 {
 	LASSERT(res);
 	LASSERT(res != LP_POISON);
-- 
1.8.3.1

[lustre-devel] [PATCH 14/30] lustre: ldlm: cond_resched in ldlm_bl_thread_main

[James Simmons]

From: Patrick Farrell <paf@cray.com>

When clearing all of the ldlm LRUs (as Cray does at the end of
a job), a ldlm_bl_work_item is generated for each namespace
and then they are placed on a list for the ldlm_bl threads to
iterate over.

If the number of namespaces greatly exceeds the number of
ldlm_bl threads, a given thread will iterate over many
namespaces without sleeping looking for work.  This can go
on for an extremely long time and result in an RCU stall.

This patch adds a cond_resched() between completing one
work item and looking for the next.  This is a fairly cheap
operation, as it will only schedule if there is an
interrupt waiting, and it will not be called too much -
Even the largest file systems have < 100 namespaces per
ldlm_bl_thread currently.

Signed-off-by: Patrick Farrell <paf@cray.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-8307
Reviewed-on: https://review.whamcloud.com/20888
Reviewed-by: Ned Bass <bass6@llnl.gov>
Reviewed-by: Ann Koehler <amk@cray.com>
Reviewed-by: Ben Evans <bevans@cray.com>
Reviewed-by: James Simmons <uja.ornl@yahoo.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
index adc96b6..a8de3d9 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
@@ -856,6 +856,12 @@ static int ldlm_bl_thread_main(void *arg)
 
 		if (rc == LDLM_ITER_STOP)
 			break;
+
+		/* If there are many namespaces, we will not sleep waiting for
+		 * work, and must do a cond_resched to avoid holding the CPU
+		 * for too long
+		 */
+		cond_resched();
 	}
 
 	atomic_dec(&blp->blp_num_threads);
-- 
1.8.3.1

[lustre-devel] [PATCH 15/30] lustre: ptlrpc: drain "ptlrpc_request_buffer_desc" objects

[James Simmons]

From: Bruno Faccini <bruno.faccini@intel.com>

Prior to this patch, new "ptlrpc_request_buffer_desc"
could be additionally allocated upon need by
ptlrpc_check_rqbd_pool(), but will never be freed
until OST umount/stop by ptlrpc_service_purge_all().
Now try to release some of them when possible.

Signed-off-by: Bruno Faccini <bruno.faccini@intel.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-9372
Reviewed-on: https://review.whamcloud.com/26752
Reviewed-by: Niu Yawei <yawei.niu@intel.com>
Reviewed-by: Henri Doreau <henri.doreau@cea.fr>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/ptlrpc/service.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ptlrpc/service.c b/drivers/staging/lustre/lustre/ptlrpc/service.c
index 79baadc..6a5a9c5 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/service.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/service.c
@@ -802,11 +802,21 @@ static void ptlrpc_server_drop_request(struct ptlrpc_request *req)
 			spin_lock(&svcpt->scp_lock);
 			/*
 			 * now all reqs including the embedded req has been
-			 * disposed, schedule request buffer for re-use.
+			 * disposed, schedule request buffer for re-use
+			 * or free it to drain some in excess.
 			 */
 			LASSERT(atomic_read(&rqbd->rqbd_req.rq_refcount) ==
 				0);
-			list_add_tail(&rqbd->rqbd_list, &svcpt->scp_rqbd_idle);
+			if (svcpt->scp_nrqbds_posted >= svc->srv_nbuf_per_group &&
+			    !test_req_buffer_pressure) {
+				/* like in ptlrpc_free_rqbd() */
+				svcpt->scp_nrqbds_total--;
+				kvfree(rqbd->rqbd_buffer);
+				kfree(rqbd);
+			} else {
+				list_add_tail(&rqbd->rqbd_list,
+					      &svcpt->scp_rqbd_idle);
+			}
 		}
 
 		spin_unlock(&svcpt->scp_lock);
-- 
1.8.3.1

[lustre-devel] [PATCH 16/30] lustre: lnet: Change sock_create() to sock_create_kern()

[James Simmons]

From: Doug Oucharek <dougso@me.com>

Changing all calls from sock_create() to sock_create_kern().

Signed-off-by: Doug Oucharek <dougso@me.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-9456
Reviewed-on: https://review.whamcloud.com/26958
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: James Simmons <uja.ornl@yahoo.com>
Reviewed-by: Olaf Weber <olaf.weber@hpe.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lnet/lnet/lib-socket.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lnet/lnet/lib-socket.c b/drivers/staging/lustre/lnet/lnet/lib-socket.c
index 6758090..d9c62d3 100644
--- a/drivers/staging/lustre/lnet/lnet/lib-socket.c
+++ b/drivers/staging/lustre/lnet/lnet/lib-socket.c
@@ -157,7 +157,7 @@
 	/* All errors are fatal except bind failure if the port is in use */
 	*fatal = 1;
 
-	rc = sock_create(PF_INET, SOCK_STREAM, 0, &sock);
+	rc = sock_create_kern(&init_net, PF_INET, SOCK_STREAM, 0, &sock);
 	*sockp = sock;
 	if (rc) {
 		CERROR("Can't create socket: %d\n", rc);
-- 
1.8.3.1

[lustre-devel] [PATCH 17/30] lustre: llite: protect ll_dentry_data modification

[James Simmons]

From: Andriy Skulysh <c17819@cray.com>

The ll_dentry_data bitfields modification should be protected by
a spinlock.

Signed-off-by: Andriy Skulysh <c17819@cray.com>
Signed-off-by: Lai Siyao <lai.siyao@whamcloud.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-9241
Seagate-bug-id: MRP-4257
Reviewed-on: https://review.whamcloud.com/26109
Reviewed-by: Niu Yawei <yawei.niu@intel.com>
Reviewed-by: Bobi Jam <bobijam@hotmail.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/llite/llite_nfs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/llite/llite_nfs.c b/drivers/staging/lustre/lustre/llite/llite_nfs.c
index c66072a..5e91e83 100644
--- a/drivers/staging/lustre/lustre/llite/llite_nfs.c
+++ b/drivers/staging/lustre/lustre/llite/llite_nfs.c
@@ -171,8 +171,9 @@ struct lustre_nfs_fid {
 	 * we came from NFS and so opencache needs to be
 	 * enabled for this one
 	 */
+	spin_lock(&result->d_lock);
 	ll_d2d(result)->lld_nfs_dentry = 1;
-
+	spin_unlock(&result->d_lock);
 	return result;
 }
 
-- 
1.8.3.1

[lustre-devel] [PATCH 18/30] lustre: llite: llite.stat_blocksize param for fixed st_blksize

[James Simmons]

From: Andrew Perepechko <c17827@cray.com>

llite.stat_blocksize is added to allow configurable st_blksize
for stat(2). The latter is treated incorrectly by some
applications. For example, glibc pre-2.25 uses this value for
stdio buffering which completely ruins performance with random
reads.

The patch changes the behaviour of getattr rather than inode
initialization so that change of the setting causes immediate
effect without the need of reclaiming existing inodes.

The patch is similar to the patch from bz # 12739 by Aurelien
Degremont.

Signed-off-by: Andrew Perepechko <c17827@cray.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-9413
Reviewed-on: https://review.whamcloud.com/26869
Reviewed-by: Alexander Zarochentsev <c17826@cray.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/llite/file.c         |  2 +-
 .../staging/lustre/lustre/llite/llite_internal.h   |  6 +++-
 drivers/staging/lustre/lustre/llite/lproc_llite.c  | 33 ++++++++++++++++++++++
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c
index b90c59c..94760eb 100644
--- a/drivers/staging/lustre/lustre/llite/file.c
+++ b/drivers/staging/lustre/lustre/llite/file.c
@@ -3267,7 +3267,7 @@ int ll_getattr(const struct path *path, struct kstat *stat,
 	stat->atime = inode->i_atime;
 	stat->mtime = inode->i_mtime;
 	stat->ctime = inode->i_ctime;
-	stat->blksize = 1 << inode->i_blkbits;
+	stat->blksize = sbi->ll_stat_blksize ?: BIT(inode->i_blkbits);
 
 	stat->nlink = inode->i_nlink;
 	stat->size = i_size_read(inode);
diff --git a/drivers/staging/lustre/lustre/llite/llite_internal.h b/drivers/staging/lustre/lustre/llite/llite_internal.h
index 9d9f623..dcb2fed 100644
--- a/drivers/staging/lustre/lustre/llite/llite_internal.h
+++ b/drivers/staging/lustre/lustre/llite/llite_internal.h
@@ -523,7 +523,11 @@ struct ll_sb_info {
 	struct root_squash_info	  ll_squash;
 	struct path		 ll_mnt;
 
-	__kernel_fsid_t		  ll_fsid;
+	/* st_blksize returned by stat(2), when non-zero */
+	unsigned int		 ll_stat_blksize;
+
+	__kernel_fsid_t		 ll_fsid;
+
 	struct kset		ll_kset;	/* sysfs object */
 	struct completion	 ll_kobj_unregister;
 };
diff --git a/drivers/staging/lustre/lustre/llite/lproc_llite.c b/drivers/staging/lustre/lustre/llite/lproc_llite.c
index f775b4f..d8ef090 100644
--- a/drivers/staging/lustre/lustre/llite/lproc_llite.c
+++ b/drivers/staging/lustre/lustre/llite/lproc_llite.c
@@ -128,6 +128,38 @@ static ssize_t blocksize_show(struct kobject *kobj, struct attribute *attr,
 }
 LUSTRE_RO_ATTR(blocksize);
 
+static ssize_t stat_blocksize_show(struct kobject *kobj, struct attribute *attr,
+				   char *buf)
+{
+	struct ll_sb_info *sbi = container_of(kobj, struct ll_sb_info,
+					      ll_kset.kobj);
+
+	return sprintf(buf, "%u\n", sbi->ll_stat_blksize);
+}
+
+static ssize_t stat_blocksize_store(struct kobject *kobj,
+				    struct attribute *attr,
+				    const char *buffer,
+				    size_t count)
+{
+	struct ll_sb_info *sbi = container_of(kobj, struct ll_sb_info,
+					      ll_kset.kobj);
+	unsigned int val;
+	int rc;
+
+	rc = kstrtouint(buffer, 10, &val);
+	if (rc)
+		return rc;
+
+	if (val != 0 && (val < PAGE_SIZE || (val & (val - 1))) != 0)
+		return -ERANGE;
+
+	sbi->ll_stat_blksize = val;
+
+	return count;
+}
+LUSTRE_RW_ATTR(stat_blocksize);
+
 static ssize_t kbytestotal_show(struct kobject *kobj, struct attribute *attr,
 				char *buf)
 {
@@ -1123,6 +1155,7 @@ static ssize_t ll_nosquash_nids_seq_write(struct file *file,
 
 static struct attribute *llite_attrs[] = {
 	&lustre_attr_blocksize.attr,
+	&lustre_attr_stat_blocksize.attr,
 	&lustre_attr_kbytestotal.attr,
 	&lustre_attr_kbytesfree.attr,
 	&lustre_attr_kbytesavail.attr,
-- 
1.8.3.1

[lustre-devel] [PATCH 19/30] lustre: llite: set sec ctx on client's inode at create time

[James Simmons]

From: Sebastien Buisson <sbuisson@ddn.com>

On client side, security context must be set on the inode of
every new dir or file that is being created.

With LL_SBI_FILE_SECCTX, security context is obtained from the
dentry thanks to a call to ll_dentry_init_security(). And it is
saved to security.xxx xattr directly on MDS side when processing
the create request. So it is only necessary to call
security_inode_notifysecctx() to set the sec ctx on the client's
inode.

Without LL_SBI_FILE_SECCTX, security context can only be obtained
from the inode, ie after the file has been created on MDS side.
So use ll_inode_init_security() that will set the sec ctx on the
client's inode, and at the same time save it on disk to
security.xxx xattr, generating an additional request to the MDS.

Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-8956
Reviewed-on: https://review.whamcloud.com/24426
Reviewed-by: Faccini Bruno <bruno.faccini@intel.com>
Reviewed-by: Jean-Baptiste Riaux <riaux.jb@intel.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/llite/dir.c   | 10 ++++-
 drivers/staging/lustre/lustre/llite/namei.c | 59 ++++++++++++++++++++++++-----
 2 files changed, 59 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/dir.c b/drivers/staging/lustre/lustre/llite/dir.c
index 26b0c2d..aa55bfd 100644
--- a/drivers/staging/lustre/lustre/llite/dir.c
+++ b/drivers/staging/lustre/lustre/llite/dir.c
@@ -42,6 +42,7 @@
 #include <linux/buffer_head.h>   /* for wait_on_buffer */
 #include <linux/pagevec.h>
 #include <linux/prefetch.h>
+#include <linux/security.h>
 
 #define DEBUG_SUBSYSTEM S_LLITE
 
@@ -468,8 +469,15 @@ static int ll_dir_setdirstripe(struct dentry *dparent, struct lmv_user_md *lump,
 
 	dentry.d_inode = inode;
 
-	if (!(sbi->ll_flags & LL_SBI_FILE_SECCTX))
+	if (sbi->ll_flags & LL_SBI_FILE_SECCTX) {
+		inode_lock(inode);
+		err = security_inode_notifysecctx(inode,
+						  op_data->op_file_secctx,
+						  op_data->op_file_secctx_size);
+		inode_unlock(inode);
+	} else {
 		err = ll_inode_init_security(&dentry, inode, parent);
+	}
 
 out_inode:
 	iput(inode);
diff --git a/drivers/staging/lustre/lustre/llite/namei.c b/drivers/staging/lustre/lustre/llite/namei.c
index ca81e0c..09cdf02 100644
--- a/drivers/staging/lustre/lustre/llite/namei.c
+++ b/drivers/staging/lustre/lustre/llite/namei.c
@@ -47,7 +47,7 @@
 #include "llite_internal.h"
 
 static int ll_create_it(struct inode *dir, struct dentry *dentry,
-			struct lookup_intent *it);
+			struct lookup_intent *it, void *secctx, u32 secctxlen);
 
 /* called from iget5_locked->find_inode() under inode_hash_lock spinlock */
 static int ll_test_inode(struct inode *inode, void *opaque)
@@ -527,7 +527,8 @@ static int ll_lookup_it_finish(struct ptlrpc_request *request,
 }
 
 static struct dentry *ll_lookup_it(struct inode *parent, struct dentry *dentry,
-				   struct lookup_intent *it)
+				   struct lookup_intent *it, void **secctx,
+				   u32 *secctxlen)
 {
 	struct lookup_intent lookup_it = { .it_op = IT_LOOKUP };
 	struct dentry *save = dentry, *retval;
@@ -590,6 +591,10 @@ static struct dentry *ll_lookup_it(struct inode *parent, struct dentry *dentry,
 			retval = ERR_PTR(rc);
 			goto out;
 		}
+		if (secctx)
+			*secctx = op_data->op_file_secctx;
+		if (secctxlen)
+			*secctxlen = op_data->op_file_secctx_size;
 	}
 
 	rc = md_intent_lock(ll_i2mdexp(parent), op_data, it, &req,
@@ -649,8 +654,16 @@ static struct dentry *ll_lookup_it(struct inode *parent, struct dentry *dentry,
 	else
 		retval = dentry;
 out:
-	if (op_data && !IS_ERR(op_data))
+	if (op_data && !IS_ERR(op_data)) {
+		if (secctx && secctxlen) {
+			/* caller needs sec ctx info, so reset it in op_data to
+			 * prevent it from being freed
+			 */
+			op_data->op_file_secctx = NULL;
+			op_data->op_file_secctx_size = 0;
+		}
 		ll_finish_md_op_data(op_data);
+	}
 
 	ptlrpc_req_finished(req);
 	return retval;
@@ -677,7 +690,7 @@ static struct dentry *ll_lookup_nd(struct inode *parent, struct dentry *dentry,
 		itp = NULL;
 	else
 		itp = &it;
-	de = ll_lookup_it(parent, dentry, itp);
+	de = ll_lookup_it(parent, dentry, itp, NULL, NULL);
 
 	if (itp)
 		ll_intent_release(itp);
@@ -694,6 +707,8 @@ static int ll_atomic_open(struct inode *dir, struct dentry *dentry,
 			  umode_t mode)
 {
 	struct lookup_intent *it;
+	void *secctx = NULL;
+	u32 secctxlen = 0;
 	struct dentry *de;
 	int rc = 0;
 
@@ -730,7 +745,7 @@ static int ll_atomic_open(struct inode *dir, struct dentry *dentry,
 	it->it_flags &= ~MDS_OPEN_FL_INTERNAL;
 
 	/* Dentry added to dcache tree in ll_lookup_it */
-	de = ll_lookup_it(dir, dentry, it);
+	de = ll_lookup_it(dir, dentry, it, &secctx, &secctxlen);
 	if (IS_ERR(de))
 		rc = PTR_ERR(de);
 	else if (de)
@@ -739,7 +754,8 @@ static int ll_atomic_open(struct inode *dir, struct dentry *dentry,
 	if (!rc) {
 		if (it_disposition(it, DISP_OPEN_CREATE)) {
 			/* Dentry instantiated in ll_create_it. */
-			rc = ll_create_it(dir, dentry, it);
+			rc = ll_create_it(dir, dentry, it, secctx, secctxlen);
+			security_release_secctx(secctx, secctxlen);
 			if (rc) {
 				/* We dget in ll_splice_alias. */
 				if (de)
@@ -828,7 +844,7 @@ static struct inode *ll_create_node(struct inode *dir, struct lookup_intent *it)
  * with d_instantiate().
  */
 static int ll_create_it(struct inode *dir, struct dentry *dentry,
-			struct lookup_intent *it)
+			struct lookup_intent *it, void *secctx, u32 secctxlen)
 {
 	struct inode *inode;
 	int rc = 0;
@@ -844,6 +860,18 @@ static int ll_create_it(struct inode *dir, struct dentry *dentry,
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
 
+	if ((ll_i2sbi(inode)->ll_flags & LL_SBI_FILE_SECCTX) && secctx) {
+		inode_lock(inode);
+		/* must be done before d_instantiate, because it calls
+		 * security_d_instantiate, which means a getxattr if security
+		 * context is not set yet
+		 */
+		rc = security_inode_notifysecctx(inode, secctx, secctxlen);
+		inode_unlock(inode);
+		if (rc)
+			return rc;
+	}
+
 	d_instantiate(dentry, inode);
 
 	if (!(ll_i2sbi(inode)->ll_flags & LL_SBI_FILE_SECCTX))
@@ -906,8 +934,6 @@ static int ll_new_node(struct inode *dir, struct dentry *dentry,
 			from_kuid(&init_user_ns, current_fsuid()),
 			from_kgid(&init_user_ns, current_fsgid()),
 			current_cap(), rdev, &request);
-	ll_finish_md_op_data(op_data);
-	op_data = NULL;
 	if (err < 0 && err != -EREMOTE)
 		goto err_exit;
 
@@ -944,6 +970,7 @@ static int ll_new_node(struct inode *dir, struct dentry *dentry,
 
 		ptlrpc_req_finished(request);
 		request = NULL;
+		ll_finish_md_op_data(op_data);
 		goto again;
 	}
 
@@ -953,6 +980,20 @@ static int ll_new_node(struct inode *dir, struct dentry *dentry,
 	if (err)
 		goto err_exit;
 
+	if (sbi->ll_flags & LL_SBI_FILE_SECCTX) {
+		inode_lock(inode);
+		/* must be done before d_instantiate, because it calls
+		 * security_d_instantiate, which means a getxattr if security
+		 * context is not set yet
+		 */
+		err = security_inode_notifysecctx(inode,
+						  op_data->op_file_secctx,
+						  op_data->op_file_secctx_size);
+		inode_unlock(inode);
+		if (err)
+			goto err_exit;
+	}
+
 	d_instantiate(dentry, inode);
 
 	if (!(sbi->ll_flags & LL_SBI_FILE_SECCTX))
-- 
1.8.3.1

[lustre-devel] [PATCH 20/30] lustre: osc: osc page lru list race

[James Simmons]

From: Bobi Jam <bobijam@hotmail.com>

In osc_lru_use() the osc page's ops_lru access is not protected, which
could race with osc_lru_del() and ensuing that
client_obd::cl_lru_in_list counter decreased twice for a single page.

Signed-off-by: Bobi Jam <bobijam@hotmail.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-9229
Reviewed-on: https://review.whamcloud.com/26086
Reviewed-by: Jinshan Xiong <jinshan.xiong@gmail.com>
Reviewed-by: Li Xi <lixi@ddn.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/osc/osc_page.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/lustre/lustre/osc/osc_page.c b/drivers/staging/lustre/lustre/osc/osc_page.c
index 20c553e..189e3e78 100644
--- a/drivers/staging/lustre/lustre/osc/osc_page.c
+++ b/drivers/staging/lustre/lustre/osc/osc_page.c
@@ -478,18 +478,20 @@ static void osc_lru_del(struct client_obd *cli, struct osc_page *opg)
 }
 
 /**
- * Delete page from LRUlist for redirty.
+ * Delete page from LRU list for redirty.
  */
 static void osc_lru_use(struct client_obd *cli, struct osc_page *opg)
 {
 	/* If page is being transferred for the first time,
 	 * ops_lru should be empty
 	 */
-	if (opg->ops_in_lru && !list_empty(&opg->ops_lru)) {
+	if (opg->ops_in_lru) {
 		spin_lock(&cli->cl_lru_list_lock);
-		__osc_lru_del(cli, opg);
+		if (!list_empty(&opg->ops_lru)) {
+			__osc_lru_del(cli, opg);
+			atomic_long_inc(&cli->cl_lru_busy);
+		}
 		spin_unlock(&cli->cl_lru_list_lock);
-		atomic_long_inc(&cli->cl_lru_busy);
 	}
 }
 
-- 
1.8.3.1

[lustre-devel] [PATCH 21/30] lustre: ldlm: use static initializer macros where possible

[James Simmons]

From: "John L. Hammond" <jhammond@whamcloud.com>

In lustre/ldlm/ replace module load time initialization of several
mutexes with static initialization using the kernel provided macros.

Signed-off-by: John L. Hammond <jhammond@whamcloud.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-9010
Reviewed-on: https://review.whamcloud.com/24824
Reviewed-by: Dmitry Eremin <dmitry.eremin@intel.com>
Reviewed-by: James Simmons <uja.ornl@yahoo.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/include/lustre_dlm.h | 3 +--
 drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c    | 6 +-----
 drivers/staging/lustre/lustre/ldlm/ldlm_resource.c | 4 ++--
 3 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_dlm.h b/drivers/staging/lustre/lustre/include/lustre_dlm.h
index a68c7a4..e2bbcaa 100644
--- a/drivers/staging/lustre/lustre/include/lustre_dlm.h
+++ b/drivers/staging/lustre/lustre/include/lustre_dlm.h
@@ -742,8 +742,7 @@ struct ldlm_lock {
 	 * The lists this could be linked into are:
 	 * waiting_locks_list (protected by waiting_locks_spinlock),
 	 * then if the lock timed out, it is moved to
-	 * expired_lock_thread.elt_expired_locks for further processing.
-	 * Protected by elt_lock.
+	 * expired_lock_list for further processing.
 	 */
 	struct list_head		l_pending_chain;
 
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
index a8de3d9..986c378 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
@@ -53,7 +53,7 @@
 module_param(ldlm_cpts, charp, 0444);
 MODULE_PARM_DESC(ldlm_cpts, "CPU partitions ldlm threads should run on");
 
-static struct mutex	ldlm_ref_mutex;
+static DEFINE_MUTEX(ldlm_ref_mutex);
 static int ldlm_refcount;
 
 static struct kobject *ldlm_kobj;
@@ -69,10 +69,6 @@ struct ldlm_cb_async_args {
 
 static struct ldlm_state *ldlm_state;
 
-#define ELT_STOPPED   0
-#define ELT_READY     1
-#define ELT_TERMINATE 2
-
 struct ldlm_bl_pool {
 	spinlock_t		blp_lock;
 
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c b/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c
index 1907a5a..bd5622d 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c
@@ -49,10 +49,10 @@
 int ldlm_srv_namespace_nr;
 int ldlm_cli_namespace_nr;
 
-struct mutex ldlm_srv_namespace_lock;
+DEFINE_MUTEX(ldlm_srv_namespace_lock);
 LIST_HEAD(ldlm_srv_namespace_list);
 
-struct mutex ldlm_cli_namespace_lock;
+DEFINE_MUTEX(ldlm_cli_namespace_lock);
 /* Client Namespaces that have active resources in them.
  * Once all resources go away, ldlm_poold moves such namespaces to the
  * inactive list
-- 
1.8.3.1

[lustre-devel] [PATCH 22/30] lustre: osc: update timestamps on write only

[James Simmons]

From: Niu Yawei <yawei.niu@intel.com>

In osc_io_submit(), we should only update timestamps on write.

Signed-off-by: Niu Yawei <yawei.niu@intel.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-9470
Reviewed-on: https://review.whamcloud.com/27348
Reviewed-by: Bobi Jam <bobijam@hotmail.com>
Reviewed-by: John L. Hammond <jhammond@whamcloud.com>
Reviewed-by: Jinshan Xiong <jinshan.xiong@gmail.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/osc/osc_io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/osc/osc_io.c b/drivers/staging/lustre/lustre/osc/osc_io.c
index 628743b..e7151ed 100644
--- a/drivers/staging/lustre/lustre/osc/osc_io.c
+++ b/drivers/staging/lustre/lustre/osc/osc_io.c
@@ -212,7 +212,7 @@ static int osc_io_submit(const struct lu_env *env,
 		result = osc_queue_sync_pages(env, osc, &list, cmd, brw_flags);
 
 	/* Update c/mtime for sync write. LU-7310 */
-	if (qout->pl_nr > 0 && !result) {
+	if (crt == CRT_WRITE && qout->pl_nr > 0 && !result) {
 		struct cl_attr *attr = &osc_env_info(env)->oti_attr;
 		struct cl_object *obj = ios->cis_obj;
 
-- 
1.8.3.1

[lustre-devel] [PATCH 23/30] lustre: osc: adds radix_tree_preload

[James Simmons]

From: Alexandr Boyko <c17825@cray.com>

The client fail with next error osc_page_init())
ASSERTION( result == 0 ) in low memory conditions.

The patch fixes the problem above by adding
radix_tree_preload.

Signed-off-by: Alexandr Boyko <c17825@cray.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-9579
Seagate-bug-id: MRP-4424
Reviewed-on: https://review.whamcloud.com/27372
Reviewed-by: Jinshan Xiong <jinshan.xiong@gmail.com>
Reviewed-by: Bobi Jam <bobijam@hotmail.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/osc/osc_page.c | 33 ++++++++++++++++------------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/lustre/lustre/osc/osc_page.c b/drivers/staging/lustre/lustre/osc/osc_page.c
index 189e3e78..ada1eda 100644
--- a/drivers/staging/lustre/lustre/osc/osc_page.c
+++ b/drivers/staging/lustre/lustre/osc/osc_page.c
@@ -256,32 +256,37 @@ int osc_page_init(const struct lu_env *env, struct cl_object *obj,
 {
 	struct osc_object *osc = cl2osc(obj);
 	struct osc_page *opg = cl_object_page_slice(obj, page);
+	struct osc_io *oio = osc_env_io(env);
 	int result;
 
 	opg->ops_from = 0;
 	opg->ops_to = PAGE_SIZE;
+	INIT_LIST_HEAD(&opg->ops_lru);
 
 	result = osc_prep_async_page(osc, opg, page->cp_vmpage,
 				     cl_offset(obj, index));
-	if (result == 0) {
-		struct osc_io *oio = osc_env_io(env);
+	if (result != 0)
+		return result;
 
-		opg->ops_srvlock = osc_io_srvlock(oio);
-		cl_page_slice_add(page, &opg->ops_cl, obj, index,
-				  &osc_page_ops);
-	}
-	INIT_LIST_HEAD(&opg->ops_lru);
+	opg->ops_srvlock = osc_io_srvlock(oio);
+	cl_page_slice_add(page, &opg->ops_cl, obj, index,
+			  &osc_page_ops);
 
 	/* reserve an LRU space for this page */
-	if (page->cp_type == CPT_CACHEABLE && result == 0) {
+	if (page->cp_type == CPT_CACHEABLE) {
 		result = osc_lru_alloc(env, osc_cli(osc), opg);
 		if (result == 0) {
-			spin_lock(&osc->oo_tree_lock);
-			result = radix_tree_insert(&osc->oo_tree, index, opg);
-			if (result == 0)
-				++osc->oo_npages;
-			spin_unlock(&osc->oo_tree_lock);
-			LASSERT(result == 0);
+			result = radix_tree_preload(GFP_KERNEL);
+			if (result == 0) {
+				spin_lock(&osc->oo_tree_lock);
+				result = radix_tree_insert(&osc->oo_tree,
+							   index, opg);
+				if (result == 0)
+					++osc->oo_npages;
+				spin_unlock(&osc->oo_tree_lock);
+
+				radix_tree_preload_end();
+			}
 		}
 	}
 
-- 
1.8.3.1

[lustre-devel] [PATCH 24/30] lustre: llite: rcu-walk check should not depend on statahead

[James Simmons]

From: Steve Guminski <stephenx.guminski@intel.com>

Moves the check for the LOOKUP_RCU flag, so that it does not depend
on the statahead setting.  The caller is now informed if rcu-walk
was requested but the filesystem does not support it, regardless
of whether statahead is enabled or disabled.

Signed-off-by: Steve Guminski <stephenx.guminski@intel.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-8891
Reviewed-on: https://review.whamcloud.com/24195
Reviewed-by: John L. Hammond <jhammond@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/llite/dcache.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/dcache.c b/drivers/staging/lustre/lustre/llite/dcache.c
index 11b82c63..ee1ba16 100644
--- a/drivers/staging/lustre/lustre/llite/dcache.c
+++ b/drivers/staging/lustre/lustre/llite/dcache.c
@@ -270,13 +270,12 @@ static int ll_revalidate_dentry(struct dentry *dentry,
 	if (lookup_flags & LOOKUP_REVAL)
 		return 0;
 
-	if (!dentry_may_statahead(dir, dentry))
-		return 1;
-
 	if (lookup_flags & LOOKUP_RCU)
 		return -ECHILD;
 
-	ll_statahead(dir, &dentry, !d_inode(dentry));
+	if (dentry_may_statahead(dir, dentry))
+		ll_statahead(dir, &dentry, !d_inode(dentry));
+
 	return 1;
 }
 
-- 
1.8.3.1

[lustre-devel] [PATCH 25/30] lustre: llite: check the return value of cl_file_inode_init()

[James Simmons]

From: Bobi Jam <bobijam@hotmail.com>

ll_update_inode() does not check the return value of
cl_file_inode_init(), and it should check.

Signed-off-by: Bobi Jam <bobijam@hotmail.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-9485
Reviewed-on: https://review.whamcloud.com/27658
Reviewed-by: Fan Yong <fan.yong@intel.com>
Reviewed-by: Jinshan Xiong <jinshan.xiong@gmail.com>
Reviewed-by: James Simmons <uja.ornl@yahoo.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/llite/llite_lib.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/llite_lib.c b/drivers/staging/lustre/lustre/llite/llite_lib.c
index b5bafc4..99d0b72 100644
--- a/drivers/staging/lustre/lustre/llite/llite_lib.c
+++ b/drivers/staging/lustre/lustre/llite/llite_lib.c
@@ -1792,13 +1792,15 @@ int ll_update_inode(struct inode *inode, struct lustre_md *md)
 	struct ll_inode_info *lli = ll_i2info(inode);
 	struct mdt_body *body = md->body;
 	struct ll_sb_info *sbi = ll_i2sbi(inode);
+	int rc = 0;
 
-	if (body->mbo_valid & OBD_MD_FLEASIZE)
-		cl_file_inode_init(inode, md);
+	if (body->mbo_valid & OBD_MD_FLEASIZE) {
+		rc = cl_file_inode_init(inode, md);
+		if (rc)
+			return rc;
+	}
 
 	if (S_ISDIR(inode->i_mode)) {
-		int rc;
-
 		rc = ll_update_lsm_md(inode, md);
 		if (rc)
 			return rc;
-- 
1.8.3.1

[lustre-devel] [PATCH 26/30] lustre: ptlrpc: add replay request into unreplied list

[James Simmons]

From: Niu Yawei <yawei.niu@intel.com>

ptlrpc_prepare_replay() may fail to add replay request into unreplied
list if the request hasn't been on replay list yet, so in
ptlrpc_replay_next() before sending replay, we'd always make sure the
replay request is on unreplied list.

Signed-off-by: Niu Yawei <yawei.niu@intel.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-9274
Reviewed-on: https://review.whamcloud.com/27920
Reviewed-by: Fan Yong <fan.yong@intel.com>
Reviewed-by: Yang Sheng <ys@whamcloud.com>
Reviewed-by: Mike Pershin <mpershin@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/ptlrpc/recover.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ptlrpc/recover.c b/drivers/staging/lustre/lustre/ptlrpc/recover.c
index 2ea0a7f..9d369f8 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/recover.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/recover.c
@@ -136,11 +136,14 @@ int ptlrpc_replay_next(struct obd_import *imp, int *inflight)
 	if (req && imp->imp_resend_replay)
 		lustre_msg_add_flags(req->rq_reqmsg, MSG_RESENT);
 
-	/* The resend replay request may have been removed from the
+	/* ptlrpc_prepare_replay() may fail to add the request into unreplied
+	 * list if the request hasn't been added to replay list then. Another
+	 * exception is that resend replay could have been removed from the
 	 * unreplied list.
 	 */
-	if (req && imp->imp_resend_replay &&
-	    list_empty(&req->rq_unreplied_list)) {
+	if (req && list_empty(&req->rq_unreplied_list)) {
+		DEBUG_REQ(D_HA, req, "resend_replay: %d, last_transno: %llu\n",
+			  imp->imp_resend_replay, last_transno);
 		ptlrpc_add_unreplied(req);
 		imp->imp_known_replied_xid = ptlrpc_known_replied_xid(imp);
 	}
@@ -149,9 +152,6 @@ int ptlrpc_replay_next(struct obd_import *imp, int *inflight)
 	spin_unlock(&imp->imp_lock);
 
 	if (req) {
-		/* The request should have been added back in unreplied list
-		 * by ptlrpc_prepare_replay().
-		 */
 		LASSERT(!list_empty(&req->rq_unreplied_list));
 
 		rc = ptlrpc_replay_req(req);
-- 
1.8.3.1

[lustre-devel] [PATCH 27/30] lustre: ptlrpc: increase sleep time in ptlrpc_request_bufs_pack()

[James Simmons]

From: Vitaly Fertman <c17818@cray.com>

schedule_timeout_uninterruptible() does not necessarily expire.
Increase the sleeping time in ptlrpc_request_bufs_pack() as 2
seconds is too short, given the 1 second sleep used in the test
suite.

Signed-off-by: Vitaly Fertman <c17818@cray.com>
Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-8062
Reviewed-on: https://review.whamcloud.com/26815
Reviewed-by: James Simmons <uja.ornl@yahoo.com>
Reviewed-by: Fan Yong <fan.yong@intel.com>
Reviewed-by: John L. Hammond <jhammond@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/ptlrpc/client.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/ptlrpc/client.c b/drivers/staging/lustre/lustre/ptlrpc/client.c
index eddb920..6d503d7 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/client.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/client.c
@@ -36,7 +36,9 @@
 #define DEBUG_SUBSYSTEM S_RPC
 
 #include <linux/libcfs/libcfs_cpu.h>
+#include <linux/delay.h>
 #include <linux/random.h>
+
 #include <obd_support.h>
 #include <obd_class.h>
 #include <lustre_lib.h>
@@ -763,7 +765,7 @@ int ptlrpc_request_bufs_pack(struct ptlrpc_request *request,
 			/* The RPC is infected, let the test change the
 			 * fail_loc
 			 */
-			schedule_timeout_uninterruptible(2 * HZ);
+			msleep(4 * MSEC_PER_SEC);
 		}
 	}
 
-- 
1.8.3.1

[lustre-devel] [PATCH 28/30] lustre: ptlrpc: free reply buffer earlier for open RPC

[James Simmons]

From: Fan Yong <fan.yong@intel.com>

It is unnecessary to keep the reply buffer for open RPC. Replay
related data has already been saved in the request buffer when
the RPC replied. If the open replay really happen, the replay
logic will alloc the reply buffer when needed.

On the other hand, the client always tries to alloc big enough
space to hold the open RPC reply since the client does not exactly
know how much data the server will reply to the client. So the reply
buffer may be quite larger than the real needed. Under such case,
keeping the large reply buffer for the open RPC will occupy a lot
of RAM as to OOM if the are too many open RPCs to be replayed.

This patch frees the reply buffer for the open RPC when only
the replay logic holds the last reference of the RPC.

Signed-off-by: Fan Yong <fan.yong@intel.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-9514
Reviewed-on: https://review.whamcloud.com/27208
Reviewed-by: Niu Yawei <yawei.niu@intel.com>
Reviewed-by: Jinshan Xiong <jinshan.xiong@gmail.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/include/lustre_net.h |  6 ++-
 drivers/staging/lustre/lustre/mdc/mdc_request.c    | 20 ++++++++-
 drivers/staging/lustre/lustre/ptlrpc/client.c      | 49 +++++++++++++++++-----
 .../staging/lustre/lustre/ptlrpc/pack_generic.c    | 26 ++++++++----
 .../staging/lustre/lustre/ptlrpc/ptlrpc_internal.h |  1 +
 5 files changed, 80 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_net.h b/drivers/staging/lustre/lustre/include/lustre_net.h
index cf630db..e755e99 100644
--- a/drivers/staging/lustre/lustre/include/lustre_net.h
+++ b/drivers/staging/lustre/lustre/include/lustre_net.h
@@ -738,7 +738,8 @@ struct ptlrpc_request {
 	/** Lock to protect request flags and some other important bits, like
 	 * rq_list
 	 */
-	spinlock_t rq_lock;
+	spinlock_t			 rq_lock;
+	spinlock_t			 rq_early_free_lock;
 	/** client-side flags are serialized by rq_lock @{ */
 	unsigned int rq_intr:1, rq_replied:1, rq_err:1,
 		rq_timedout:1, rq_resend:1, rq_restart:1,
@@ -770,7 +771,8 @@ struct ptlrpc_request {
 		 */
 		rq_allow_replay:1,
 		/* bulk request, sent to server, but uncommitted */
-		rq_unstable:1;
+		rq_unstable:1,
+		rq_early_free_repbuf:1; /* free reply buffer in advance */
 	/** @} */
 
 	/** server-side flags @{ */
diff --git a/drivers/staging/lustre/lustre/mdc/mdc_request.c b/drivers/staging/lustre/lustre/mdc/mdc_request.c
index 37bf486..2108877 100644
--- a/drivers/staging/lustre/lustre/mdc/mdc_request.c
+++ b/drivers/staging/lustre/lustre/mdc/mdc_request.c
@@ -573,8 +573,15 @@ void mdc_replay_open(struct ptlrpc_request *req)
 
 	body = req_capsule_server_get(&req->rq_pill, &RMF_MDT_BODY);
 
+	spin_lock(&req->rq_lock);
 	och = mod->mod_och;
-	if (och) {
+	if (och && och->och_fh.cookie)
+		req->rq_early_free_repbuf = 1;
+	else
+		req->rq_early_free_repbuf = 0;
+	spin_unlock(&req->rq_lock);
+
+	if (req->rq_early_free_repbuf) {
 		struct lustre_handle *file_fh;
 
 		LASSERT(och->och_magic == OBD_CLIENT_HANDLE_MAGIC);
@@ -585,6 +592,7 @@ void mdc_replay_open(struct ptlrpc_request *req)
 		old = *file_fh;
 		*file_fh = body->mbo_handle;
 	}
+
 	close_req = mod->mod_close_req;
 	if (close_req) {
 		__u32 opc = lustre_msg_get_opc(close_req->rq_reqmsg);
@@ -595,8 +603,9 @@ void mdc_replay_open(struct ptlrpc_request *req)
 					       &RMF_MDT_EPOCH);
 		LASSERT(epoch);
 
-		if (och)
+		if (req->rq_early_free_repbuf)
 			LASSERT(!memcmp(&old, &epoch->mio_handle, sizeof(old)));
+
 		DEBUG_REQ(D_HA, close_req, "updating close body with new fh");
 		epoch->mio_handle = body->mbo_handle;
 	}
@@ -677,6 +686,7 @@ int mdc_set_open_replay_data(struct obd_export *exp,
 		mod->mod_open_req = open_req;
 		open_req->rq_cb_data = mod;
 		open_req->rq_commit_cb = mdc_commit_open;
+		open_req->rq_early_free_repbuf = 1;
 		spin_unlock(&open_req->rq_lock);
 	}
 
@@ -731,6 +741,12 @@ static int mdc_clear_open_replay_data(struct obd_export *exp,
 
 	LASSERT(mod != LP_POISON);
 	LASSERT(mod->mod_open_req);
+
+	spin_lock(&mod->mod_open_req->rq_lock);
+	if (mod->mod_och)
+		mod->mod_och->och_fh.cookie = 0;
+	mod->mod_open_req->rq_early_free_repbuf = 0;
+	spin_unlock(&mod->mod_open_req->rq_lock);
 	mdc_free_open(mod);
 
 	mod->mod_och = NULL;
diff --git a/drivers/staging/lustre/lustre/ptlrpc/client.c b/drivers/staging/lustre/lustre/ptlrpc/client.c
index 6d503d7..8fafc8d 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/client.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/client.c
@@ -2413,28 +2413,54 @@ static void __ptlrpc_free_req(struct ptlrpc_request *request, int locked)
  * Drops one reference count for request \a request.
  * \a locked set indicates that caller holds import imp_lock.
  * Frees the request when reference count reaches zero.
+ *
+ * RETURN 1	the request is freed
+ * RETURN 0	some others still hold references on the request
  */
 static int __ptlrpc_req_finished(struct ptlrpc_request *request, int locked)
 {
+	int count;
+
 	if (!request)
 		return 1;
 
-	if (request == LP_POISON ||
-	    request->rq_reqmsg == LP_POISON) {
-		CERROR("dereferencing freed request (bug 575)\n");
-		LBUG();
-		return 1;
-	}
+	LASSERT(request != LP_POISON);
+	LASSERT(request->rq_reqmsg != LP_POISON);
 
 	DEBUG_REQ(D_INFO, request, "refcount now %u",
 		  atomic_read(&request->rq_refcount) - 1);
 
-	if (atomic_dec_and_test(&request->rq_refcount)) {
-		__ptlrpc_free_req(request, locked);
-		return 1;
+	spin_lock(&request->rq_lock);
+	count = atomic_dec_return(&request->rq_refcount);
+	LASSERTF(count >= 0, "Invalid ref count %d\n", count);
+
+	/* For open RPC, the client does not know the EA size (LOV, ACL, and
+	 * so on) before replied, then the client has to reserve very large
+	 * reply buffer. Such buffer will not be released until the RPC freed.
+	 * Since The open RPC is replayable, we need to keep it in the replay
+	 * list until close. If there are a lot of files opened concurrently,
+	 * then the client may be OOM.
+	 *
+	 * If fact, it is unnecessary to keep reply buffer for open replay,
+	 * related EAs have already been saved via mdc_save_lovea() before
+	 * coming here. So it is safe to free the reply buffer some earlier
+	 * before releasing the RPC to avoid client OOM. LU-9514
+	 */
+	if (count == 1 && request->rq_early_free_repbuf && request->rq_repbuf) {
+		spin_lock(&request->rq_early_free_lock);
+		sptlrpc_cli_free_repbuf(request);
+		request->rq_repbuf = NULL;
+		request->rq_repbuf_len = 0;
+		request->rq_repdata = NULL;
+		request->rq_reqdata_len = 0;
+		spin_unlock(&request->rq_early_free_lock);
 	}
+	spin_unlock(&request->rq_lock);
 
-	return 0;
+	if (!count)
+		__ptlrpc_free_req(request, locked);
+
+	return !count;
 }
 
 /**
@@ -2920,6 +2946,9 @@ int ptlrpc_replay_req(struct ptlrpc_request *req)
 	DEBUG_REQ(D_HA, req, "REPLAY");
 
 	atomic_inc(&req->rq_import->imp_replay_inflight);
+	spin_lock(&req->rq_lock);
+	req->rq_early_free_repbuf = 0;
+	spin_unlock(&req->rq_lock);
 	ptlrpc_request_addref(req); /* ptlrpcd needs a ref */
 
 	ptlrpcd_add_req(req);
diff --git a/drivers/staging/lustre/lustre/ptlrpc/pack_generic.c b/drivers/staging/lustre/lustre/ptlrpc/pack_generic.c
index 86a64a6..96d0377 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/pack_generic.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/pack_generic.c
@@ -2169,7 +2169,8 @@ static inline int req_ptlrpc_body_swabbed(struct ptlrpc_request *req)
 
 static inline int rep_ptlrpc_body_swabbed(struct ptlrpc_request *req)
 {
-	LASSERT(req->rq_repmsg);
+	if (unlikely(!req->rq_repmsg))
+		return 0;
 
 	switch (req->rq_repmsg->lm_magic) {
 	case LUSTRE_MSG_MAGIC_V2:
@@ -2181,19 +2182,30 @@ static inline int rep_ptlrpc_body_swabbed(struct ptlrpc_request *req)
 }
 
 void _debug_req(struct ptlrpc_request *req,
-		struct libcfs_debug_msg_data *msgdata,
-		const char *fmt, ...)
+		struct libcfs_debug_msg_data *msgdata, const char *fmt, ...)
 {
-	int req_ok = req->rq_reqmsg != NULL;
-	int rep_ok = req->rq_repmsg != NULL;
+	bool req_ok = req->rq_reqmsg != NULL;
+	bool rep_ok = false;
 	lnet_nid_t nid = LNET_NID_ANY;
+	int rep_flags = -1;
+	int rep_status = -1;
 	va_list args;
 
+	spin_lock(&req->rq_early_free_lock);
+	if (req->rq_repmsg)
+		rep_ok = true;
+
 	if (ptlrpc_req_need_swab(req)) {
 		req_ok = req_ok && req_ptlrpc_body_swabbed(req);
 		rep_ok = rep_ok && rep_ptlrpc_body_swabbed(req);
 	}
 
+	if (rep_ok) {
+		rep_flags = lustre_msg_get_flags(req->rq_repmsg);
+		rep_status = lustre_msg_get_status(req->rq_repmsg);
+	}
+	spin_unlock(&req->rq_early_free_lock);
+
 	if (req->rq_import && req->rq_import->imp_connection)
 		nid = req->rq_import->imp_connection->c_peer.nid;
 	else if (req->rq_export && req->rq_export->exp_connection)
@@ -2218,9 +2230,7 @@ void _debug_req(struct ptlrpc_request *req,
 			   atomic_read(&req->rq_refcount),
 			   DEBUG_REQ_FLAGS(req),
 			   req_ok ? lustre_msg_get_flags(req->rq_reqmsg) : -1,
-			   rep_ok ? lustre_msg_get_flags(req->rq_repmsg) : -1,
-			   req->rq_status,
-			   rep_ok ? lustre_msg_get_status(req->rq_repmsg) : -1);
+			   rep_flags, req->rq_status, rep_status);
 	va_end(args);
 }
 EXPORT_SYMBOL(_debug_req);
diff --git a/drivers/staging/lustre/lustre/ptlrpc/ptlrpc_internal.h b/drivers/staging/lustre/lustre/ptlrpc/ptlrpc_internal.h
index 134b742..0e4a215 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/ptlrpc_internal.h
+++ b/drivers/staging/lustre/lustre/ptlrpc/ptlrpc_internal.h
@@ -309,6 +309,7 @@ static inline void ptlrpc_reqset_put(struct ptlrpc_request_set *set)
 static inline void ptlrpc_req_comm_init(struct ptlrpc_request *req)
 {
 	spin_lock_init(&req->rq_lock);
+	spin_lock_init(&req->rq_early_free_lock);
 	atomic_set(&req->rq_refcount, 1);
 	INIT_LIST_HEAD(&req->rq_list);
 	INIT_LIST_HEAD(&req->rq_replay_list);
-- 
1.8.3.1

[lustre-devel] [PATCH 29/30] lustre: libcfs: use save_stack_trace for stack dump

[James Simmons]

From: Yang Sheng <ys@whamcloud.com>

The function libcfs_debug_dumpstack() was removed due to it being
x86 specific upstream. This restores this functionality using
save_stack_trace_tsk for stack trace dumps. This will be used for
the restored ptlrpc watchdog that will be landed in the future.

Signed-off-by: Yang Sheng <ys@whamcloud.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-11062
Reviewed-on: https://review.whamcloud.com/32952
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: James Simmons <uja.ornl@yahoo.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 .../lustre/include/linux/libcfs/libcfs_debug.h     |  1 +
 drivers/staging/lustre/lnet/libcfs/debug.c         | 46 ++++++++++++++++++++--
 drivers/staging/lustre/lustre/ldlm/ldlm_lock.c     |  2 +-
 drivers/staging/lustre/lustre/osc/osc_object.c     |  2 +-
 4 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_debug.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_debug.h
index 17534a7..27a3b12 100644
--- a/drivers/staging/lustre/include/linux/libcfs/libcfs_debug.h
+++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_debug.h
@@ -58,6 +58,7 @@
 
 int libcfs_debug_mask2str(char *str, int size, int mask, int is_subsys);
 int libcfs_debug_str2mask(int *mask, const char *str, int is_subsys);
+void libcfs_debug_dumpstack(struct task_struct *tsk);
 
 /* Has there been an LBUG? */
 extern unsigned int libcfs_catastrophe;
diff --git a/drivers/staging/lustre/lnet/libcfs/debug.c b/drivers/staging/lustre/lnet/libcfs/debug.c
index dd06a4c..90d756c 100644
--- a/drivers/staging/lustre/lnet/libcfs/debug.c
+++ b/drivers/staging/lustre/lnet/libcfs/debug.c
@@ -42,6 +42,8 @@
 #include <linux/ctype.h>
 #include <linux/libcfs/libcfs_string.h>
 #include <linux/kthread.h>
+#include <linux/stacktrace.h>
+#include <linux/utsname.h>
 #include "tracefile.h"
 
 static char debug_file_name[1024];
@@ -435,17 +437,55 @@ void __noreturn lbug_with_loc(struct libcfs_debug_msg_data *msgdata)
 		/* not reached */
 	}
 
-	dump_stack();
-	if (!libcfs_panic_on_lbug)
-		libcfs_debug_dumplog();
+	libcfs_debug_dumpstack(NULL);
 	if (libcfs_panic_on_lbug)
 		panic("LBUG");
+	else
+		libcfs_debug_dumplog();
 	set_current_state(TASK_UNINTERRUPTIBLE);
 	while (1)
 		schedule();
 }
 EXPORT_SYMBOL(lbug_with_loc);
 
+#ifdef CONFIG_STACKTRACE
+#define MAX_ST_ENTRIES 100
+static DEFINE_SPINLOCK(st_lock);
+
+static void libcfs_call_trace(struct task_struct *tsk)
+{
+	static unsigned long entries[MAX_ST_ENTRIES];
+	struct stack_trace trace;
+
+	trace.nr_entries = 0;
+	trace.max_entries = MAX_ST_ENTRIES;
+	trace.entries = entries;
+	trace.skip = 0;
+
+	spin_lock(&st_lock);
+	pr_info("Pid: %d, comm: %.20s %s %s\n", tsk->pid, tsk->comm,
+		init_utsname()->release, init_utsname()->version);
+	pr_info("Call Trace:\n");
+	save_stack_trace_tsk(tsk, &trace);
+	print_stack_trace(&trace, 0);
+	spin_unlock(&st_lock);
+}
+#else /* !CONFIG_STACKTRACE */
+static void libcfs_call_trace(struct task_struct *tsk)
+{
+	if (tsk == current)
+		dump_stack();
+	else
+		CWARN("can't show stack: kernel doesn't export show_task\n");
+}
+#endif /* !CONFIG_STACKTRACE */
+
+void libcfs_debug_dumpstack(struct task_struct *tsk)
+{
+	libcfs_call_trace(tsk ?: current);
+}
+EXPORT_SYMBOL(libcfs_debug_dumpstack);
+
 static int panic_notifier(struct notifier_block *self, unsigned long unused1,
 			  void *unused2)
 {
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
index 3cbdc81..bc6b122 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
@@ -1503,7 +1503,7 @@ int ldlm_fill_lvb(struct ldlm_lock *lock, struct req_capsule *pill,
 		break;
 	default:
 		LDLM_ERROR(lock, "Unknown LVB type: %d", lock->l_lvb_type);
-		dump_stack();
+		libcfs_debug_dumpstack(NULL);
 		return -EINVAL;
 	}
 
diff --git a/drivers/staging/lustre/lustre/osc/osc_object.c b/drivers/staging/lustre/lustre/osc/osc_object.c
index b9bf2b8..a86d4c2 100644
--- a/drivers/staging/lustre/lustre/osc/osc_object.c
+++ b/drivers/staging/lustre/lustre/osc/osc_object.c
@@ -399,7 +399,7 @@ static void osc_req_attr_set(const struct lu_env *env, struct cl_object *obj,
 				osc_export(cl2osc(obj))->exp_obd->obd_namespace,
 				NULL, resname, LDLM_EXTENT, 0);
 			ldlm_resource_dump(D_ERROR, res);
-
+			libcfs_debug_dumpstack(NULL);
 			LBUG();
 		}
 
-- 
1.8.3.1

[lustre-devel] [PATCH 30/30] lustre: clio: fix spare bit handling

[James Simmons]

Expanded testing of the earlier patch that changed lustre so not
to use the spare bits in iattr.ia_valid exposed a bug with HSM.
The bugs was that ll_setattr_raw() was ignoring the changes to
ctime.

Signed-off-by: James Simmons <uja.ornl@yahoo.com>
WC-bug-id: https://jira.whamcloud.com/browse/LU-10030
Reviewed-on: https://review.whamcloud.com/32825
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Bobi Jam <bobijam@hotmail.com>
Signed-off-by: James Simmons <jsimmons@infradead.org>
---
 drivers/staging/lustre/lustre/llite/llite_lib.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/llite_lib.c b/drivers/staging/lustre/lustre/llite/llite_lib.c
index 99d0b72..3de6e886 100644
--- a/drivers/staging/lustre/lustre/llite/llite_lib.c
+++ b/drivers/staging/lustre/lustre/llite/llite_lib.c
@@ -1529,7 +1529,8 @@ int ll_setattr_raw(struct dentry *dentry, struct iattr *attr,
 	}
 
 	/* We mark all of the fields "set" so MDS/OST does not re-set them */
-	if (attr->ia_valid & ATTR_CTIME) {
+	if (!(xvalid & OP_ATTR_CTIME_SET) &&
+	    attr->ia_valid & ATTR_CTIME) {
 		attr->ia_ctime = current_time(inode);
 		xvalid |= OP_ATTR_CTIME_SET;
 	}
@@ -1584,9 +1585,9 @@ int ll_setattr_raw(struct dentry *dentry, struct iattr *attr,
 		goto out;
 	}
 
-	if (attr->ia_valid & (ATTR_SIZE |
-			      ATTR_ATIME | ATTR_ATIME_SET |
-			      ATTR_MTIME | ATTR_MTIME_SET)) {
+	if (attr->ia_valid & (ATTR_SIZE | ATTR_ATIME | ATTR_ATIME_SET |
+			      ATTR_MTIME | ATTR_MTIME_SET | ATTR_CTIME) ||
+	    xvalid & OP_ATTR_CTIME_SET) {
 		/* For truncate and utimes sending attributes to OSTs, setting
 		 * mtime/atime to the past will be performed under PW [0:EOF]
 		 * extent lock (new_size:EOF for truncate).  It may seem
-- 
1.8.3.1

[lustre-devel] [PATCH 11/30] lustre: mdc: allow setting readdir RPC size parameter

[NeilBrown]

On Mon, Sep 17 2018, James Simmons wrote:

> From: Andreas Dilger <adilger@whamcloud.com>
>
> Allow the mdc.*.max_pages_per_rpc tunable to set the MDS bulk
> readdir RPC size, rather than always using the default 1MB RPC
> size. The tunable is set in the MDC, as it should be, rather
> than in the llite superblock, which requires extra code just to
> get it up from the MDC's connect_data only to send it back down.
> The RPC size could be tuned independently if different types of
> MDSes are used (e.g. local vs. remote).
>
> Remove the md_op_data.op_max_pages and ll_sb_info.ll_md_brw_pages
> fields that previously were used to pass the readdir size from
> llite to mdc_read_page(). Reorder some 32-bit fields in md_op_data
> to avoid struct holes.
>
> Move osc's max_pages_per_rpc_store() to obdclass along with
> max_pages_per_rpc_show().
>
> Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
> WC-bug-id: https://jira.whamcloud.com/browse/LU-3308
> Reviewed-on: https://review.whamcloud.com/26088
> Reviewed-by: Niu Yawei <yawei.niu@intel.com>
> Reviewed-by: Lai Siyao <lai.siyao@whamcloud.com>
> Reviewed-by: Oleg Drokin <green@whamcloud.com>
> Signed-off-by: James Simmons <jsimmons@infradead.org>
> ---
>  .../staging/lustre/lustre/include/lprocfs_status.h |  5 ++
>  drivers/staging/lustre/lustre/include/lustre_net.h |  8 ++--
>  drivers/staging/lustre/lustre/include/obd.h        |  5 +-
>  drivers/staging/lustre/lustre/llite/dir.c          | 12 +----
>  drivers/staging/lustre/lustre/llite/lcommon_cl.c   | 19 +++-----
>  .../staging/lustre/lustre/llite/llite_internal.h   |  2 -
>  drivers/staging/lustre/lustre/llite/llite_lib.c    |  5 --
>  drivers/staging/lustre/lustre/llite/llite_nfs.c    |  1 -
>  drivers/staging/lustre/lustre/llite/statahead.c    |  4 --
>  drivers/staging/lustre/lustre/mdc/lproc_mdc.c      | 20 +-------
>  drivers/staging/lustre/lustre/mdc/mdc_request.c    | 13 +++---
>  .../lustre/lustre/obdclass/lprocfs_status.c        | 54 ++++++++++++++++++++++
>  drivers/staging/lustre/lustre/osc/lproc_osc.c      | 46 ------------------
>  drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c    |  2 +-
>  14 files changed, 82 insertions(+), 114 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/include/lprocfs_status.h b/drivers/staging/lustre/lustre/include/lprocfs_status.h
> index c841aba..5da26e3 100644
> --- a/drivers/staging/lustre/lustre/include/lprocfs_status.h
> +++ b/drivers/staging/lustre/lustre/include/lprocfs_status.h
> @@ -584,6 +584,11 @@ ssize_t lustre_attr_store(struct kobject *kobj, struct attribute *attr,
>  
>  extern const struct sysfs_ops lustre_sysfs_ops;
>  
> +ssize_t max_pages_per_rpc_show(struct kobject *kobj, struct attribute *attr,
> +			       char *buf);
> +ssize_t max_pages_per_rpc_store(struct kobject *kobj, struct attribute *attr,
> +				const char *buffer, size_t count);
> +
>  struct root_squash_info;
>  int lprocfs_wr_root_squash(const char __user *buffer, unsigned long count,
>  			   struct root_squash_info *squash, char *name);
> diff --git a/drivers/staging/lustre/lustre/include/lustre_net.h b/drivers/staging/lustre/lustre/include/lustre_net.h
> index 2dbd208..cf630db 100644
> --- a/drivers/staging/lustre/lustre/include/lustre_net.h
> +++ b/drivers/staging/lustre/lustre/include/lustre_net.h
> @@ -104,15 +104,15 @@
>   * currently supported maximum between peers at connect via ocd_brw_size.
>   */
>  #define PTLRPC_MAX_BRW_BITS	(LNET_MTU_BITS + PTLRPC_BULK_OPS_BITS)
> -#define PTLRPC_MAX_BRW_SIZE	(1 << PTLRPC_MAX_BRW_BITS)
> +#define PTLRPC_MAX_BRW_SIZE	BIT(PTLRPC_MAX_BRW_BITS)
>  #define PTLRPC_MAX_BRW_PAGES	(PTLRPC_MAX_BRW_SIZE >> PAGE_SHIFT)
>  
> -#define ONE_MB_BRW_SIZE		(1 << LNET_MTU_BITS)
> -#define MD_MAX_BRW_SIZE		(1 << LNET_MTU_BITS)
> +#define ONE_MB_BRW_SIZE		BIT(LNET_MTU_BITS)
> +#define MD_MAX_BRW_SIZE		BIT(LNET_MTU_BITS)
>  #define MD_MAX_BRW_PAGES	(MD_MAX_BRW_SIZE >> PAGE_SHIFT)
>  #define DT_MAX_BRW_SIZE		PTLRPC_MAX_BRW_SIZE
>  #define DT_MAX_BRW_PAGES	(DT_MAX_BRW_SIZE >> PAGE_SHIFT)
> -#define OFD_MAX_BRW_SIZE	(1 << LNET_MTU_BITS)
> +#define OFD_MAX_BRW_SIZE	BIT(LNET_MTU_BITS)

Argg!!  No!!  Names are important.
"SIZE" means the value is a size, a number.  The bit-representation is
largely irrelevant, it is the number that is important.
BIT(x) returns a single bit - lots of zeros and just one '1' bit.  This
is not a number, it is a bit pattern.

So settings FOO_SIZE to BIT(bar) is wrong.  It is a type error.  It uses
a bit pattern when a number is expected.  The C compiler won't notice, but I will.

When I apply this (which probably won't be until next week), I'll just
remove this section of the patch.


>  
>  /* When PAGE_SIZE is a constant, we can check our arithmetic here with cpp! */
>  # if ((PTLRPC_MAX_BRW_PAGES & (PTLRPC_MAX_BRW_PAGES - 1)) != 0)
> diff --git a/drivers/staging/lustre/lustre/include/obd.h b/drivers/staging/lustre/lustre/include/obd.h
> index 329bae9..50e97b4 100644
> --- a/drivers/staging/lustre/lustre/include/obd.h
> +++ b/drivers/staging/lustre/lustre/include/obd.h
> @@ -717,11 +717,11 @@ struct md_op_data {
>  	struct lu_fid	   op_fid3; /* 2 extra fids to find conflicting */
>  	struct lu_fid	   op_fid4; /* to the operation locks. */
>  	u32			op_mds;  /* what mds server open will go to */
> +	u32			op_mode;
>  	struct lustre_handle    op_handle;
>  	s64			op_mod_time;
>  	const char	     *op_name;
>  	size_t			op_namelen;
> -	__u32		   op_mode;
>  	struct lmv_stripe_md   *op_mea1;
>  	struct lmv_stripe_md   *op_mea2;
>  	__u32		   op_suppgids[2];
> @@ -746,9 +746,6 @@ struct md_op_data {
>  	/* Used by readdir */
>  	__u64		   op_offset;
>  
> -	/* Used by readdir */
> -	__u32			op_max_pages;
> -
>  	/* used to transfer info between the stacks of MD client
>  	 * see enum op_cli_flags
>  	 */
> diff --git a/drivers/staging/lustre/lustre/llite/dir.c b/drivers/staging/lustre/lustre/llite/dir.c
> index f352fab..26b0c2d 100644
> --- a/drivers/staging/lustre/lustre/llite/dir.c
> +++ b/drivers/staging/lustre/lustre/llite/dir.c
> @@ -231,18 +231,11 @@ int ll_dir_read(struct inode *inode, __u64 *ppos, struct md_op_data *op_data,
>  			__u64	  ino;
>  
>  			hash = le64_to_cpu(ent->lde_hash);
> -			if (hash < pos)
> -				/*
> -				 * Skip until we find target hash
> -				 * value.
> -				 */
> +			if (hash < pos) /* Skip until we find target hash */
>  				continue;

If these comments are really needed (i++; /* increment i  */), then
they should be:
 if (hash < pos)
       /* skip until we find target hash */
       continue;


>  
>  			namelen = le16_to_cpu(ent->lde_namelen);
> -			if (namelen == 0)
> -				/*
> -				 * Skip dummy record.
> -				 */
> +			if (namelen == 0) /* Skip dummy record. */
>  				continue;
>  
>  			if (is_api32 && is_hash64)
> @@ -351,7 +344,6 @@ static int ll_readdir(struct file *filp, struct dir_context *ctx)
>  			}
>  		}
>  	}
> -	op_data->op_max_pages = sbi->ll_md_brw_pages;
>  	ctx->pos = pos;
>  	rc = ll_dir_read(inode, &pos, op_data, ctx);
>  	pos = ctx->pos;
> diff --git a/drivers/staging/lustre/lustre/llite/lcommon_cl.c b/drivers/staging/lustre/lustre/llite/lcommon_cl.c
> index 6c9fe49..d3b0445 100644
> --- a/drivers/staging/lustre/lustre/llite/lcommon_cl.c
> +++ b/drivers/staging/lustre/lustre/llite/lcommon_cl.c
> @@ -267,27 +267,22 @@ void cl_inode_fini(struct inode *inode)
>  /**
>   * build inode number from passed @fid
>   */
> -__u64 cl_fid_build_ino(const struct lu_fid *fid, int api32)
> +u64 cl_fid_build_ino(const struct lu_fid *fid, int api32)
>  {
>  	if (BITS_PER_LONG == 32 || api32)
>  		return fid_flatten32(fid);
> -	else
> -		return fid_flatten(fid);
> +
> +	return fid_flatten(fid);

I preferred that as it was - it makes the either/or nature more obvious.

NeilBrown


>  }
>  
>  /**
>   * build inode generation from passed @fid.  If our FID overflows the 32-bit
>   * inode number then return a non-zero generation to distinguish them.
>   */
> -__u32 cl_fid_build_gen(const struct lu_fid *fid)
> +u32 cl_fid_build_gen(const struct lu_fid *fid)
>  {
> -	__u32 gen;
> -
> -	if (fid_is_igif(fid)) {
> -		gen = lu_igif_gen(fid);
> -		return gen;
> -	}
> +	if (fid_is_igif(fid))
> +		return lu_igif_gen(fid);
>  
> -	gen = fid_flatten(fid) >> 32;
> -	return gen;
> +	return fid_flatten(fid) >> 32;
>  }
> diff --git a/drivers/staging/lustre/lustre/llite/llite_internal.h b/drivers/staging/lustre/lustre/llite/llite_internal.h
> index bf679c9..9d9f623 100644
> --- a/drivers/staging/lustre/lustre/llite/llite_internal.h
> +++ b/drivers/staging/lustre/lustre/llite/llite_internal.h
> @@ -490,8 +490,6 @@ struct ll_sb_info {
>  	unsigned int	      ll_namelen;
>  	const struct file_operations	*ll_fop;
>  
> -	unsigned int		  ll_md_brw_pages; /* readdir pages per RPC */
> -
>  	struct lu_site	   *ll_site;
>  	struct cl_device	 *ll_cl;
>  	/* Statistics */
> diff --git a/drivers/staging/lustre/lustre/llite/llite_lib.c b/drivers/staging/lustre/lustre/llite/llite_lib.c
> index f0fe21f..b5bafc4 100644
> --- a/drivers/staging/lustre/lustre/llite/llite_lib.c
> +++ b/drivers/staging/lustre/lustre/llite/llite_lib.c
> @@ -343,11 +343,6 @@ static int client_common_fill_super(struct super_block *sb, char *md, char *dt)
>  	if (data->ocd_connect_flags & OBD_CONNECT_64BITHASH)
>  		sbi->ll_flags |= LL_SBI_64BIT_HASH;
>  
> -	if (data->ocd_connect_flags & OBD_CONNECT_BRW_SIZE)
> -		sbi->ll_md_brw_pages = data->ocd_brw_size >> PAGE_SHIFT;
> -	else
> -		sbi->ll_md_brw_pages = 1;
> -
>  	if (data->ocd_connect_flags & OBD_CONNECT_LAYOUTLOCK)
>  		sbi->ll_flags |= LL_SBI_LAYOUT_LOCK;
>  
> diff --git a/drivers/staging/lustre/lustre/llite/llite_nfs.c b/drivers/staging/lustre/lustre/llite/llite_nfs.c
> index 9efb20e..c66072a 100644
> --- a/drivers/staging/lustre/lustre/llite/llite_nfs.c
> +++ b/drivers/staging/lustre/lustre/llite/llite_nfs.c
> @@ -261,7 +261,6 @@ static int ll_get_name(struct dentry *dentry, char *name,
>  		goto out;
>  	}
>  
> -	op_data->op_max_pages = ll_i2sbi(dir)->ll_md_brw_pages;
>  	inode_lock(dir);
>  	rc = ll_dir_read(dir, &pos, op_data, &lgd.ctx);
>  	inode_unlock(dir);
> diff --git a/drivers/staging/lustre/lustre/llite/statahead.c b/drivers/staging/lustre/lustre/llite/statahead.c
> index ea2a002..1ad308c 100644
> --- a/drivers/staging/lustre/lustre/llite/statahead.c
> +++ b/drivers/staging/lustre/lustre/llite/statahead.c
> @@ -961,8 +961,6 @@ static int ll_statahead_thread(void *arg)
>  		goto out;
>  	}
>  
> -	op_data->op_max_pages = ll_i2sbi(dir)->ll_md_brw_pages;
> -
>  	while (pos != MDS_DIR_END_OFF && sai->sai_task) {
>  		struct lu_dirpage *dp;
>  		struct lu_dirent  *ent;
> @@ -1215,8 +1213,6 @@ static int is_first_dirent(struct inode *dir, struct dentry *dentry)
>  	/**
>  	 * FIXME choose the start offset of the readdir
>  	 */
> -	op_data->op_max_pages = ll_i2sbi(dir)->ll_md_brw_pages;
> -
>  	page = ll_get_dir_page(dir, op_data, pos);
>  
>  	while (1) {
> diff --git a/drivers/staging/lustre/lustre/mdc/lproc_mdc.c b/drivers/staging/lustre/lustre/mdc/lproc_mdc.c
> index 3bff8b5..a205c61 100644
> --- a/drivers/staging/lustre/lustre/mdc/lproc_mdc.c
> +++ b/drivers/staging/lustre/lustre/mdc/lproc_mdc.c
> @@ -134,6 +134,8 @@ static ssize_t max_mod_rpcs_in_flight_store(struct kobject *kobj,
>  }
>  LUSTRE_RW_ATTR(max_mod_rpcs_in_flight);
>  
> +LUSTRE_RW_ATTR(max_pages_per_rpc);
> +
>  #define mdc_conn_uuid_show conn_uuid_show
>  LUSTRE_RO_ATTR(mdc_conn_uuid);
>  
> @@ -165,24 +167,6 @@ static ssize_t mdc_rpc_stats_seq_write(struct file *file,
>  LPROC_SEQ_FOPS_RO_TYPE(mdc, timeouts);
>  LPROC_SEQ_FOPS_RO_TYPE(mdc, state);
>  
> -/*
> - * Note: below sysfs entry is provided, but not currently in use, instead
> - * sbi->sb_md_brw_size is used, the per obd variable should be used
> - * when DNE is enabled, and dir pages are managed in MDC layer.
> - * Don't forget to enable sysfs store function then.
> - */
> -static ssize_t max_pages_per_rpc_show(struct kobject *kobj,
> -				      struct attribute *attr,
> -				      char *buf)
> -{
> -	struct obd_device *dev = container_of(kobj, struct obd_device,
> -					      obd_kset.kobj);
> -	struct client_obd *cli = &dev->u.cli;
> -
> -	return sprintf(buf, "%d\n", cli->cl_max_pages_per_rpc);
> -}
> -LUSTRE_RO_ATTR(max_pages_per_rpc);
> -
>  LPROC_SEQ_FOPS_RW_TYPE(mdc, import);
>  LPROC_SEQ_FOPS_RW_TYPE(mdc, pinger_recov);
>  
> diff --git a/drivers/staging/lustre/lustre/mdc/mdc_request.c b/drivers/staging/lustre/lustre/mdc/mdc_request.c
> index 116e973..37bf486 100644
> --- a/drivers/staging/lustre/lustre/mdc/mdc_request.c
> +++ b/drivers/staging/lustre/lustre/mdc/mdc_request.c
> @@ -1166,17 +1166,17 @@ static int mdc_read_page_remote(void *data, struct page *page0)
>  	struct page **page_pool;
>  	struct page *page;
>  	struct lu_dirpage *dp;
> -	int rd_pgs = 0; /* number of pages read actually */
> -	int npages;
>  	struct md_op_data *op_data = rp->rp_mod;
>  	struct ptlrpc_request *req;
> -	int max_pages = op_data->op_max_pages;
>  	struct inode *inode;
>  	struct lu_fid *fid;
> -	int i;
> +	int rd_pgs = 0; /* number of pages read actually */
> +	int max_pages;
> +	int npages;
>  	int rc;
> +	int i;
>  
> -	LASSERT(max_pages > 0 && max_pages <= PTLRPC_MAX_BRW_PAGES);
> +	max_pages = rp->rp_exp->exp_obd->u.cli.cl_max_pages_per_rpc;
>  	inode = op_data->op_data;
>  	fid = &op_data->op_fid1;
>  	LASSERT(inode);
> @@ -1200,8 +1200,7 @@ static int mdc_read_page_remote(void *data, struct page *page0)
>  	if (!rc) {
>  		int lu_pgs = req->rq_bulk->bd_nob_transferred;
>  
> -		rd_pgs = (req->rq_bulk->bd_nob_transferred +
> -			  PAGE_SIZE - 1) >> PAGE_SHIFT;
> +		rd_pgs = (lu_pgs + PAGE_SIZE - 1) >> PAGE_SHIFT;
>  		lu_pgs >>= LU_PAGE_SHIFT;
>  		LASSERT(!(req->rq_bulk->bd_nob_transferred & ~LU_PAGE_MASK));
>  
> diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> index 84e5a8c..a540abb 100644
> --- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> @@ -1785,3 +1785,57 @@ ssize_t lustre_attr_store(struct kobject *kobj, struct attribute *attr,
>  	.store = lustre_attr_store,
>  };
>  EXPORT_SYMBOL_GPL(lustre_sysfs_ops);
> +
> +ssize_t max_pages_per_rpc_show(struct kobject *kobj, struct attribute *attr,
> +			       char *buf)
> +{
> +	struct obd_device *dev = container_of(kobj, struct obd_device,
> +					      obd_kset.kobj);
> +	struct client_obd *cli = &dev->u.cli;
> +
> +	return sprintf(buf, "%d\n", cli->cl_max_pages_per_rpc);
> +}
> +EXPORT_SYMBOL(max_pages_per_rpc_show);
> +
> +ssize_t max_pages_per_rpc_store(struct kobject *kobj, struct attribute *attr,
> +				const char *buffer, size_t count)
> +{
> +	struct obd_device *dev = container_of(kobj, struct obd_device,
> +					      obd_kset.kobj);
> +	struct client_obd *cli = &dev->u.cli;
> +	struct obd_connect_data *ocd;
> +	unsigned long long val;
> +	int chunk_mask;
> +	int rc;
> +
> +	rc = kstrtoull(buffer, 10, &val);
> +	if (rc)
> +		return rc;
> +
> +	/* if the max_pages is specified in bytes, convert to pages */
> +	if (val >= ONE_MB_BRW_SIZE)
> +		val >>= PAGE_SHIFT;
> +
> +	rc = lprocfs_climp_check(dev);
> +	if (rc)
> +		return rc;
> +
> +	ocd = &cli->cl_import->imp_connect_data;
> +	chunk_mask = ~((1 << (cli->cl_chunkbits - PAGE_SHIFT)) - 1);
> +	/* max_pages_per_rpc must be chunk aligned */
> +	val = (val + ~chunk_mask) & chunk_mask;
> +	if (!val || (ocd->ocd_brw_size &&
> +		     val > ocd->ocd_brw_size >> PAGE_SHIFT)) {
> +		up_read(&dev->u.cli.cl_sem);
> +		return -ERANGE;
> +	}
> +
> +	spin_lock(&cli->cl_loi_list_lock);
> +	cli->cl_max_pages_per_rpc = val;
> +	client_adjust_max_dirty(cli);
> +	spin_unlock(&cli->cl_loi_list_lock);
> +
> +	up_read(&dev->u.cli.cl_sem);
> +	return count;
> +}
> +EXPORT_SYMBOL(max_pages_per_rpc_store);
> diff --git a/drivers/staging/lustre/lustre/osc/lproc_osc.c b/drivers/staging/lustre/lustre/osc/lproc_osc.c
> index cd28295..bf576f1 100644
> --- a/drivers/staging/lustre/lustre/osc/lproc_osc.c
> +++ b/drivers/staging/lustre/lustre/osc/lproc_osc.c
> @@ -570,52 +570,6 @@ static ssize_t destroys_in_flight_show(struct kobject *kobj,
>  		       atomic_read(&obd->u.cli.cl_destroy_in_flight));
>  }
>  LUSTRE_RO_ATTR(destroys_in_flight);
> -
> -static ssize_t max_pages_per_rpc_show(struct kobject *kobj,
> -				      struct attribute *attr,
> -				      char *buf)
> -{
> -	struct obd_device *dev = container_of(kobj, struct obd_device,
> -					      obd_kset.kobj);
> -	struct client_obd *cli = &dev->u.cli;
> -
> -	return sprintf(buf, "%d\n", cli->cl_max_pages_per_rpc);
> -}
> -
> -static ssize_t max_pages_per_rpc_store(struct kobject *kobj,
> -				       struct attribute *attr,
> -				       const char *buffer,
> -				       size_t count)
> -{
> -	struct obd_device *dev = container_of(kobj, struct obd_device,
> -					      obd_kset.kobj);
> -	struct client_obd *cli = &dev->u.cli;
> -	struct obd_connect_data *ocd = &cli->cl_import->imp_connect_data;
> -	int chunk_mask, rc;
> -	unsigned long long val;
> -
> -	rc = kstrtoull(buffer, 10, &val);
> -	if (rc)
> -		return rc;
> -
> -	/* if the max_pages is specified in bytes, convert to pages */
> -	if (val >= ONE_MB_BRW_SIZE)
> -		val >>= PAGE_SHIFT;
> -
> -	chunk_mask = ~((1 << (cli->cl_chunkbits - PAGE_SHIFT)) - 1);
> -	/* max_pages_per_rpc must be chunk aligned */
> -	val = (val + ~chunk_mask) & chunk_mask;
> -	if (!val || (ocd->ocd_brw_size &&
> -		     val > ocd->ocd_brw_size >> PAGE_SHIFT)) {
> -		return -ERANGE;
> -	}
> -	spin_lock(&cli->cl_loi_list_lock);
> -	cli->cl_max_pages_per_rpc = val;
> -	client_adjust_max_dirty(cli);
> -	spin_unlock(&cli->cl_loi_list_lock);
> -
> -	return count;
> -}
>  LUSTRE_RW_ATTR(max_pages_per_rpc);
>  
>  static int osc_unstable_stats_seq_show(struct seq_file *m, void *v)
> diff --git a/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c b/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c
> index 625b952..3d336d9 100644
> --- a/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c
> +++ b/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c
> @@ -232,7 +232,7 @@ static unsigned long enc_pools_shrink_count(struct shrinker *s,
>  	}
>  
>  	LASSERT(page_pools.epp_idle_idx <= IDLE_IDX_MAX);
> -	return max((int)page_pools.epp_free_pages - PTLRPC_MAX_BRW_PAGES, 0) *
> +	return max(page_pools.epp_free_pages - PTLRPC_MAX_BRW_PAGES, 0UL) *
>  		(IDLE_IDX_MAX - page_pools.epp_idle_idx) / IDLE_IDX_MAX;
>  }
>  
> -- 
> 1.8.3.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180918/eb49df23/attachment.sig>

[lustre-devel] [PATCH 11/30] lustre: mdc: allow setting readdir RPC size parameter

[Andreas Dilger]

On Sep 18, 2018, at 09:14, NeilBrown <neilb@suse.com> wrote:
> 
> On Mon, Sep 17 2018, James Simmons wrote:
> 
>> From: Andreas Dilger <adilger@whamcloud.com>
>> 
>> diff --git a/drivers/staging/lustre/lustre/include/lustre_net.h b/drivers/staging/lustre/lustre/include/lustre_net.h
>> index 2dbd208..cf630db 100644
>> --- a/drivers/staging/lustre/lustre/include/lustre_net.h
>> +++ b/drivers/staging/lustre/lustre/include/lustre_net.h
>> @@ -104,15 +104,15 @@
>>  * currently supported maximum between peers at connect via ocd_brw_size.
>>  */
>> #define PTLRPC_MAX_BRW_BITS	(LNET_MTU_BITS + PTLRPC_BULK_OPS_BITS)
>> -#define PTLRPC_MAX_BRW_SIZE	(1 << PTLRPC_MAX_BRW_BITS)
>> +#define PTLRPC_MAX_BRW_SIZE	BIT(PTLRPC_MAX_BRW_BITS)
>> #define PTLRPC_MAX_BRW_PAGES	(PTLRPC_MAX_BRW_SIZE >> PAGE_SHIFT)
>> 
>> -#define ONE_MB_BRW_SIZE		(1 << LNET_MTU_BITS)
>> -#define MD_MAX_BRW_SIZE		(1 << LNET_MTU_BITS)
>> +#define ONE_MB_BRW_SIZE		BIT(LNET_MTU_BITS)
>> +#define MD_MAX_BRW_SIZE		BIT(LNET_MTU_BITS)
>> #define MD_MAX_BRW_PAGES	(MD_MAX_BRW_SIZE >> PAGE_SHIFT)
>> #define DT_MAX_BRW_SIZE		PTLRPC_MAX_BRW_SIZE
>> #define DT_MAX_BRW_PAGES	(DT_MAX_BRW_SIZE >> PAGE_SHIFT)
>> -#define OFD_MAX_BRW_SIZE	(1 << LNET_MTU_BITS)
>> +#define OFD_MAX_BRW_SIZE	BIT(LNET_MTU_BITS)
> 
> Argg!!  No!!  Names are important.
> "SIZE" means the value is a size, a number.  The bit-representation is
> largely irrelevant, it is the number that is important.
> BIT(x) returns a single bit - lots of zeros and just one '1' bit.  This
> is not a number, it is a bit pattern.
> 
> So settings FOO_SIZE to BIT(bar) is wrong.  It is a type error.  It uses
> a bit pattern when a number is expected.  The C compiler won't notice, but I will.
> 
> When I apply this (which probably won't be until next week), I'll just
> remove this section of the patch.

Just to confirm, my original patch didn't have these BIT() macros in it,
and I agree with your statements, so I'm fine with you removing them again.

>> diff --git a/drivers/staging/lustre/lustre/llite/lcommon_cl.c b/drivers/staging/lustre/lustre/llite/lcommon_cl.c
>> index 6c9fe49..d3b0445 100644
>> --- a/drivers/staging/lustre/lustre/llite/lcommon_cl.c
>> +++ b/drivers/staging/lustre/lustre/llite/lcommon_cl.c
>> @@ -267,27 +267,22 @@ void cl_inode_fini(struct inode *inode)
>> /**
>>  * build inode number from passed @fid
>>  */
>> -__u64 cl_fid_build_ino(const struct lu_fid *fid, int api32)
>> +u64 cl_fid_build_ino(const struct lu_fid *fid, int api32)
>> {
>> 	if (BITS_PER_LONG == 32 || api32)
>> 		return fid_flatten32(fid);
>> -	else
>> -		return fid_flatten(fid);
>> +
>> +	return fid_flatten(fid);
> 
> I preferred that as it was - it makes the either/or nature more obvious.

Kernel style generally recommends no "else" after a return, and checkpatch.pl will complain in this case.

Cheers, Andreas
---
Andreas Dilger
Principal Lustre Architect
Whamcloud







-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 235 bytes
Desc: Message signed with OpenPGP
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180920/8d434fe1/attachment.sig>

[lustre-devel] [PATCH 01/30] lustre: lnd: resolve IP query code in LND drivers

[NeilBrown]

On Mon, Sep 17 2018, James Simmons wrote:

> The recent IP querying code that landed to both ksocklnd and
> ko2iblnd have some bugs that prevent proper setup. The first
> bug in both drivers is that the IP address of ifa_local is in
> big endian format so on little endian systems the IP address
> is incorrect. Calling ntohl() on ifa_local gets the real IP
> address. The second bug located in ko2iblnd is that in_dev is
> always NULL. Add the call __in_dev_get_rtnl() to get in_dev
> and use that information to query the IP address.

Thanks!
I've added:
Fixes: 10e138e41a43 ("lustre: o2iblnd: get IP address more directly.")
Fixes: f703f71afd98 ("lustre: socklnd:  use for_each_netdev() instead of lnet_ipif_enumerate()")

and applied this.

NeilBrown


>
> Signed-off-by: James Simmons <jsimmons@infradead.org>
> ---
>  drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c | 12 +++++++++---
>  drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c |  2 +-
>  2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> index 15953e4..6874e53 100644
> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> @@ -2468,8 +2468,7 @@ static struct kib_dev *kiblnd_create_dev(char *ifname)
>  	flags = dev_get_flags(netdev);
>  	if (!(flags & IFF_UP)) {
>  		CERROR("Can't query IPoIB interface %s: it's down\n", ifname);
> -		rtnl_unlock();
> -		return NULL;
> +		goto unlock;
>  	}
>  
>  	dev = kzalloc(sizeof(*dev), GFP_NOFS);
> @@ -2481,9 +2480,16 @@ static struct kib_dev *kiblnd_create_dev(char *ifname)
>  	INIT_LIST_HEAD(&dev->ibd_nets);
>  	INIT_LIST_HEAD(&dev->ibd_list); /* not yet in kib_devs */
>  	INIT_LIST_HEAD(&dev->ibd_fail_list);
> +
> +	in_dev = __in_dev_get_rtnl(netdev);
> +	if (!in_dev) {
> +		kfree(dev);
> +		goto unlock;
> +	}
> +
>  	for_primary_ifa(in_dev)
>  		if (strcmp(ifa->ifa_label, ifname) == 0) {
> -			dev->ibd_ifip = ifa->ifa_local;
> +			dev->ibd_ifip = ntohl(ifa->ifa_local);
>  			break;
>  		}
>  	endfor_ifa(in_dev);
> diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
> index 5b81040..750a7ce 100644
> --- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
> +++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c
> @@ -2589,7 +2589,7 @@ static int ksocknal_push(struct lnet_ni *ni, struct lnet_process_id id)
>  		}
>  		for_primary_ifa(in_dev)
>  			if (strcmp(ifa->ifa_label, name) == 0) {
> -				ksi->ksni_ipaddr = ifa->ifa_local;
> +				ksi->ksni_ipaddr = ntohl(ifa->ifa_local);
>  				ksi->ksni_netmask = ifa->ifa_mask;
>  				strlcpy(ksi->ksni_name,
>  					name, sizeof(ksi->ksni_name));
> -- 
> 1.8.3.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180924/08db0d01/attachment.sig>

[lustre-devel] [PATCH 08/30] lustre: mgc: Remove unnecessary checks for config_log_put()

[NeilBrown]

On Mon, Sep 17 2018, James Simmons wrote:

> From: Steve Guminski <stephenx.guminski@intel.com>
>
> Make config_log_put() check if its parameter is NULL, which makes
> it is unnecessary to perform the check prior to calling it.
> This patch removes the redundant checks.
>
> Signed-off-by: Steve Guminski <stephenx.guminski@intel.com>
> WC-bug-id: https://jira.whamcloud.com/browse/LU-9152
> Reviewed-on: https://review.whamcloud.com/25854
> Reviewed-by: Bob Glossman <bob.glossman@intel.com>
> Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
> Reviewed-by: James Simmons <uja.ornl@yahoo.com>
> Signed-off-by: James Simmons <jsimmons@infradead.org>
> ---
>  drivers/staging/lustre/lustre/mgc/mgc_request.c | 36 +++++++++++--------------
>  1 file changed, 16 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
> index 4552cc5..e6f8d9e 100644
> --- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
> +++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
> @@ -131,6 +131,9 @@ static int config_log_get(struct config_llog_data *cld)
>   */
>  static void config_log_put(struct config_llog_data *cld)
>  {
> +	if (unlikely(!cld))
> +		return;
> +

I've removed the "unlikely()" call.

Partly because use of likely/unlikely is discouraged where the
difference cannot be measured (or clearly justified some other way),
and partly because .....

> @@ -387,6 +387,9 @@ struct config_llog_data *do_config_log_add(struct obd_device *obd,
>  
>  static inline void config_mark_cld_stop(struct config_llog_data *cld)
>  {
> +	if (!cld)
> +		return;
> +

.... there is no "unlikely()" here, and I like consistency.

Thanks,
NeilBrown


>  	mutex_lock(&cld->cld_lock);
>  	spin_lock(&config_list_lock);
>  	cld->cld_stopping = 1;
> @@ -436,18 +439,13 @@ static int config_log_end(char *logname, struct config_llog_instance *cfg)
>  	cld->cld_sptlrpc = NULL;
>  	mutex_unlock(&cld->cld_lock);
>  
> -	if (cld_recover) {
> -		config_mark_cld_stop(cld_recover);
> -		config_log_put(cld_recover);
> -	}
> +	config_mark_cld_stop(cld_recover);
> +	config_log_put(cld_recover);
>  
> -	if (cld_params) {
> -		config_mark_cld_stop(cld_params);
> -		config_log_put(cld_params);
> -	}
> +	config_mark_cld_stop(cld_params);
> +	config_log_put(cld_params);
>  
> -	if (cld_sptlrpc)
> -		config_log_put(cld_sptlrpc);
> +	config_log_put(cld_sptlrpc);
>  
>  	/* drop the ref from the find */
>  	config_log_put(cld);
> @@ -593,8 +591,7 @@ static int mgc_requeue_thread(void *data)
>  			cld->cld_lostlock = 0;
>  			spin_unlock(&config_list_lock);
>  
> -			if (cld_prev)
> -				config_log_put(cld_prev);
> +			config_log_put(cld_prev);
>  			cld_prev = cld;
>  
>  			if (likely(!(rq_state & RQ_STOP))) {
> @@ -606,8 +603,7 @@ static int mgc_requeue_thread(void *data)
>  			}
>  		}
>  		spin_unlock(&config_list_lock);
> -		if (cld_prev)
> -			config_log_put(cld_prev);
> +		config_log_put(cld_prev);
>  
>  		/* Wait a bit to see if anyone else needs a requeue */
>  		wait_event_idle(rq_waitq, rq_state & (RQ_NOW | RQ_STOP));
> -- 
> 1.8.3.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180924/14be13eb/attachment.sig>

[lustre-devel] [PATCH 11/30] lustre: mdc: allow setting readdir RPC size parameter

[NeilBrown]

On Thu, Sep 20 2018, Andreas Dilger wrote:

> On Sep 18, 2018, at 09:14, NeilBrown <neilb@suse.com> wrote:
>> 
>> On Mon, Sep 17 2018, James Simmons wrote:
>> 
>>> From: Andreas Dilger <adilger@whamcloud.com>
>>> 
>>> diff --git a/drivers/staging/lustre/lustre/include/lustre_net.h b/drivers/staging/lustre/lustre/include/lustre_net.h
>>> index 2dbd208..cf630db 100644
>>> --- a/drivers/staging/lustre/lustre/include/lustre_net.h
>>> +++ b/drivers/staging/lustre/lustre/include/lustre_net.h
>>> @@ -104,15 +104,15 @@
>>>  * currently supported maximum between peers at connect via ocd_brw_size.
>>>  */
>>> #define PTLRPC_MAX_BRW_BITS	(LNET_MTU_BITS + PTLRPC_BULK_OPS_BITS)
>>> -#define PTLRPC_MAX_BRW_SIZE	(1 << PTLRPC_MAX_BRW_BITS)
>>> +#define PTLRPC_MAX_BRW_SIZE	BIT(PTLRPC_MAX_BRW_BITS)
>>> #define PTLRPC_MAX_BRW_PAGES	(PTLRPC_MAX_BRW_SIZE >> PAGE_SHIFT)
>>> 
>>> -#define ONE_MB_BRW_SIZE		(1 << LNET_MTU_BITS)
>>> -#define MD_MAX_BRW_SIZE		(1 << LNET_MTU_BITS)
>>> +#define ONE_MB_BRW_SIZE		BIT(LNET_MTU_BITS)
>>> +#define MD_MAX_BRW_SIZE		BIT(LNET_MTU_BITS)
>>> #define MD_MAX_BRW_PAGES	(MD_MAX_BRW_SIZE >> PAGE_SHIFT)
>>> #define DT_MAX_BRW_SIZE		PTLRPC_MAX_BRW_SIZE
>>> #define DT_MAX_BRW_PAGES	(DT_MAX_BRW_SIZE >> PAGE_SHIFT)
>>> -#define OFD_MAX_BRW_SIZE	(1 << LNET_MTU_BITS)
>>> +#define OFD_MAX_BRW_SIZE	BIT(LNET_MTU_BITS)
>> 
>> Argg!!  No!!  Names are important.
>> "SIZE" means the value is a size, a number.  The bit-representation is
>> largely irrelevant, it is the number that is important.
>> BIT(x) returns a single bit - lots of zeros and just one '1' bit.  This
>> is not a number, it is a bit pattern.
>> 
>> So settings FOO_SIZE to BIT(bar) is wrong.  It is a type error.  It uses
>> a bit pattern when a number is expected.  The C compiler won't notice, but I will.
>> 
>> When I apply this (which probably won't be until next week), I'll just
>> remove this section of the patch.
>
> Just to confirm, my original patch didn't have these BIT() macros in it,
> and I agree with your statements, so I'm fine with you removing them
> again.

Good.  They are gone.


>
>>> diff --git a/drivers/staging/lustre/lustre/llite/lcommon_cl.c b/drivers/staging/lustre/lustre/llite/lcommon_cl.c
>>> index 6c9fe49..d3b0445 100644
>>> --- a/drivers/staging/lustre/lustre/llite/lcommon_cl.c
>>> +++ b/drivers/staging/lustre/lustre/llite/lcommon_cl.c
>>> @@ -267,27 +267,22 @@ void cl_inode_fini(struct inode *inode)
>>> /**
>>>  * build inode number from passed @fid
>>>  */
>>> -__u64 cl_fid_build_ino(const struct lu_fid *fid, int api32)
>>> +u64 cl_fid_build_ino(const struct lu_fid *fid, int api32)
>>> {
>>> 	if (BITS_PER_LONG == 32 || api32)
>>> 		return fid_flatten32(fid);
>>> -	else
>>> -		return fid_flatten(fid);
>>> +
>>> +	return fid_flatten(fid);
>> 
>> I preferred that as it was - it makes the either/or nature more obvious.
>
> Kernel style generally recommends no "else" after a return, and checkpatch.pl will complain in this case.

I just ran
 checkpatch.pl --file drivers/staging/lustre/lustre/llite/lcommon_cl.c
without this patch applied, and it didn't complain.
I've removed this section of the patch because it seems to be unrelated
to the rest of the patch, and because I don't like it.

Thanks,
NeilBrown



>
> Cheers, Andreas
> ---
> Andreas Dilger
> Principal Lustre Architect
> Whamcloud
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180924/a702ecf6/attachment.sig>

[lustre-devel] [PATCH 12/30] lustre: kuc: initialize kkuc_groups at module init time

[NeilBrown]

On Mon, Sep 17 2018, James Simmons wrote:

> From: "John L. Hammond" <jhammond@whamcloud.com>
>
> Some kkuc functions use kkuc_groups[group].next == NULL to test for an
> empty group list. This is incorrect if the group was previously added
> to but not empty. Remove all next == NULL tests and use module load
> time initialization of the kkuc_groups array.
>
> Signed-off-by: John L. Hammond <jhammond@whamcloud.com>
> WC-bug-id: https://jira.whamcloud.com/browse/LU-9306
> Reviewed-on: https://review.whamcloud.com/26883
> Reviewed-by: Frank Zago <fzago@cray.com>
> Reviewed-by: Faccini Bruno <bruno.faccini@intel.com>
> Reviewed-by: Henri Doreau <henri.doreau@cea.fr>
> Reviewed-by: Quentin Bouget <quentin.bouget@cea.fr>
> Reviewed-by: Oleg Drokin <green@whamcloud.com>
> Signed-off-by: James Simmons <jsimmons@infradead.org>
> ---
>  .../lustre/lustre/include/lustre_kernelcomm.h      |  1 +
>  drivers/staging/lustre/lustre/obdclass/class_obd.c |  3 ++
>  .../staging/lustre/lustre/obdclass/kernelcomm.c    | 38 +++++++++++++++-------
>  3 files changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/include/lustre_kernelcomm.h b/drivers/staging/lustre/lustre/include/lustre_kernelcomm.h
> index 2b3fa84..8e3a990 100644
> --- a/drivers/staging/lustre/lustre/include/lustre_kernelcomm.h
> +++ b/drivers/staging/lustre/lustre/include/lustre_kernelcomm.h
> @@ -45,6 +45,7 @@
>  typedef int (*libcfs_kkuc_cb_t)(void *data, void *cb_arg);
>  
>  /* Kernel methods */
> +void libcfs_kkuc_init(void);
>  int libcfs_kkuc_msg_put(struct file *fp, void *payload);
>  int libcfs_kkuc_group_put(unsigned int group, void *payload);
>  int libcfs_kkuc_group_add(struct file *fp, int uid, unsigned int group,
> diff --git a/drivers/staging/lustre/lustre/obdclass/class_obd.c b/drivers/staging/lustre/lustre/obdclass/class_obd.c
> index eabaafe..2d23608 100644
> --- a/drivers/staging/lustre/lustre/obdclass/class_obd.c
> +++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c
> @@ -42,6 +42,7 @@
>  #include <obd_class.h>
>  #include <uapi/linux/lnet/lnetctl.h>
>  #include <lustre_debug.h>
> +#include <lustre_kernelcomm.h>
>  #include <lprocfs_status.h>
>  #include <linux/list.h>
>  #include <cl_object.h>
> @@ -664,6 +665,8 @@ static int __init obdclass_init(void)
>  	if (err)
>  		return err;
>  
> +	libcfs_kkuc_init();
> +
>  	err = obd_zombie_impexp_init();
>  	if (err)
>  		return err;
> diff --git a/drivers/staging/lustre/lustre/obdclass/kernelcomm.c b/drivers/staging/lustre/lustre/obdclass/kernelcomm.c
> index 304288d..0fcfecf 100644
> --- a/drivers/staging/lustre/lustre/obdclass/kernelcomm.c
> +++ b/drivers/staging/lustre/lustre/obdclass/kernelcomm.c
> @@ -96,10 +96,23 @@ struct kkuc_reg {
>  	char		 kr_data[0];
>  };
>  
> -static struct list_head kkuc_groups[KUC_GRP_MAX + 1] = {};
> +static struct list_head kkuc_groups[KUC_GRP_MAX + 1];
>  /* Protect message sending against remove and adds */
>  static DECLARE_RWSEM(kg_sem);
>  
> +static inline bool libcfs_kkuc_group_is_valid(int group)
> +{
> +	return 0 <= group && group < ARRAY_SIZE(kkuc_groups);
> +}

libcfs_kkuc_group_is_valid() is only ever passed an unsigned int.
So I've changed it to expect one, and removed the comparison against 0
which is now pointless.

Thanks,
NeilBrown


> +
> +void libcfs_kkuc_init(void)
> +{
> +	int group;
> +
> +	for (group = 0; group < ARRAY_SIZE(kkuc_groups); group++)
> +		INIT_LIST_HEAD(&kkuc_groups[group]);
> +}
> +
>  /** Add a receiver to a broadcast group
>   * @param filp pipe to write into
>   * @param uid identifier for this receiver
> @@ -111,7 +124,7 @@ int libcfs_kkuc_group_add(struct file *filp, int uid, unsigned int group,
>  {
>  	struct kkuc_reg *reg;
>  
> -	if (group > KUC_GRP_MAX) {
> +	if (!libcfs_kkuc_group_is_valid(group)) {
>  		CDEBUG(D_WARNING, "Kernelcomm: bad group %d\n", group);
>  		return -EINVAL;
>  	}
> @@ -130,8 +143,6 @@ int libcfs_kkuc_group_add(struct file *filp, int uid, unsigned int group,
>  	memcpy(reg->kr_data, data, data_len);
>  
>  	down_write(&kg_sem);
> -	if (!kkuc_groups[group].next)
> -		INIT_LIST_HEAD(&kkuc_groups[group]);
>  	list_add(&reg->kr_chain, &kkuc_groups[group]);
>  	up_write(&kg_sem);
>  
> @@ -145,8 +156,10 @@ int libcfs_kkuc_group_rem(int uid, unsigned int group)
>  {
>  	struct kkuc_reg *reg, *next;
>  
> -	if (!kkuc_groups[group].next)
> -		return 0;
> +	if (!libcfs_kkuc_group_is_valid(group)) {
> +		CDEBUG(D_WARNING, "Kernelcomm: bad group %d\n", group);
> +		return -EINVAL;
> +	}
>  
>  	if (!uid) {
>  		/* Broadcast a shutdown message */
> @@ -182,9 +195,14 @@ int libcfs_kkuc_group_put(unsigned int group, void *payload)
>  	int rc = 0;
>  	int one_success = 0;
>  
> +	if (!libcfs_kkuc_group_is_valid(group)) {
> +		CDEBUG(D_WARNING, "Kernelcomm: bad group %d\n", group);
> +		return -EINVAL;
> +	}
> +
>  	down_write(&kg_sem);
>  
> -	if (unlikely(!kkuc_groups[group].next) ||
> +	if (unlikely(list_empty(&kkuc_groups[group])) ||
>  	    unlikely(OBD_FAIL_CHECK(OBD_FAIL_MDS_HSM_CT_REGISTER_NET))) {
>  		/* no agent have fully registered, CDT will retry */
>  		up_write(&kg_sem);
> @@ -227,15 +245,11 @@ int libcfs_kkuc_group_foreach(unsigned int group, libcfs_kkuc_cb_t cb_func,
>  	struct kkuc_reg *reg;
>  	int rc = 0;
>  
> -	if (group > KUC_GRP_MAX) {
> +	if (!libcfs_kkuc_group_is_valid(group)) {
>  		CDEBUG(D_WARNING, "Kernelcomm: bad group %d\n", group);
>  		return -EINVAL;
>  	}
>  
> -	/* no link for this group */
> -	if (!kkuc_groups[group].next)
> -		return 0;
> -
>  	down_read(&kg_sem);
>  	list_for_each_entry(reg, &kkuc_groups[group], kr_chain) {
>  		if (reg->kr_fp)
> -- 
> 1.8.3.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180924/dd16318d/attachment.sig>

[lustre-devel] [PATCH 17/30] lustre: llite: protect ll_dentry_data modification

[NeilBrown]

On Mon, Sep 17 2018, James Simmons wrote:

> From: Andriy Skulysh <c17819@cray.com>
>
> The ll_dentry_data bitfields modification should be protected by
> a spinlock.
>
> Signed-off-by: Andriy Skulysh <c17819@cray.com>
> Signed-off-by: Lai Siyao <lai.siyao@whamcloud.com>
> WC-bug-id: https://jira.whamcloud.com/browse/LU-9241
> Seagate-bug-id: MRP-4257
> Reviewed-on: https://review.whamcloud.com/26109
> Reviewed-by: Niu Yawei <yawei.niu@intel.com>
> Reviewed-by: Bobi Jam <bobijam@hotmail.com>
> Reviewed-by: Oleg Drokin <green@whamcloud.com>
> Signed-off-by: James Simmons <jsimmons@infradead.org>
> ---
>  drivers/staging/lustre/lustre/llite/llite_nfs.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/lustre/lustre/llite/llite_nfs.c b/drivers/staging/lustre/lustre/llite/llite_nfs.c
> index c66072a..5e91e83 100644
> --- a/drivers/staging/lustre/lustre/llite/llite_nfs.c
> +++ b/drivers/staging/lustre/lustre/llite/llite_nfs.c
> @@ -171,8 +171,9 @@ struct lustre_nfs_fid {
>  	 * we came from NFS and so opencache needs to be
>  	 * enabled for this one
>  	 */
> +	spin_lock(&result->d_lock);
>  	ll_d2d(result)->lld_nfs_dentry = 1;
> -
> +	spin_unlock(&result->d_lock);
>  	return result;
>  }

This is a bit weird....
I agree that having a spinlock is needed as the compiler does an R/M/W
to update the bitfield, and that could race with updats to lld_invalid
(and I think that is a good arguement to avoid bitfields in shared structures).

However lld_nfs_dentry is never used.  The only use was removed by

Commit: c1b66fccf986 ("staging: lustre: fid: do open-by-fid by default")

The corresponding commit upstream is

Commit: cb85c0364fd8 ("LU-3544 fid: do open-by-fid by default")

and that commit doesn't remove the use of lld_nfs_dentry.  Maybe because
lld_nfs_dentry didn't exist then - it wasn't added until later
by 65d0add6057b138.

So do we need to bring lld_nfs_dentry back?

Thanks,
NeilBrown
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180924/9c40f448/attachment-0001.sig>

[lustre-devel] [PATCH 18/30] lustre: llite: llite.stat_blocksize param for fixed st_blksize

[NeilBrown]

On Mon, Sep 17 2018, James Simmons wrote:

> From: Andrew Perepechko <c17827@cray.com>
>
> llite.stat_blocksize is added to allow configurable st_blksize
> for stat(2). The latter is treated incorrectly by some
> applications. For example, glibc pre-2.25 uses this value for
> stdio buffering which completely ruins performance with random
> reads.
>
> The patch changes the behaviour of getattr rather than inode
> initialization so that change of the setting causes immediate
> effect without the need of reclaiming existing inodes.
>
> The patch is similar to the patch from bz # 12739 by Aurelien
> Degremont.
>
> Signed-off-by: Andrew Perepechko <c17827@cray.com>
> WC-bug-id: https://jira.whamcloud.com/browse/LU-9413
> Reviewed-on: https://review.whamcloud.com/26869
> Reviewed-by: Alexander Zarochentsev <c17826@cray.com>
> Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
> Reviewed-by: Oleg Drokin <green@whamcloud.com>
> Signed-off-by: James Simmons <jsimmons@infradead.org>
> ---
>  drivers/staging/lustre/lustre/llite/file.c         |  2 +-
>  .../staging/lustre/lustre/llite/llite_internal.h   |  6 +++-
>  drivers/staging/lustre/lustre/llite/lproc_llite.c  | 33 ++++++++++++++++++++++
>  3 files changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c
> index b90c59c..94760eb 100644
> --- a/drivers/staging/lustre/lustre/llite/file.c
> +++ b/drivers/staging/lustre/lustre/llite/file.c
> @@ -3267,7 +3267,7 @@ int ll_getattr(const struct path *path, struct kstat *stat,
>  	stat->atime = inode->i_atime;
>  	stat->mtime = inode->i_mtime;
>  	stat->ctime = inode->i_ctime;
> -	stat->blksize = 1 << inode->i_blkbits;
> +	stat->blksize = sbi->ll_stat_blksize ?: BIT(inode->i_blkbits);

I replaced the BIT() with (1<< ).

Thanks,
NeilBrown


>  
>  	stat->nlink = inode->i_nlink;
>  	stat->size = i_size_read(inode);
> diff --git a/drivers/staging/lustre/lustre/llite/llite_internal.h b/drivers/staging/lustre/lustre/llite/llite_internal.h
> index 9d9f623..dcb2fed 100644
> --- a/drivers/staging/lustre/lustre/llite/llite_internal.h
> +++ b/drivers/staging/lustre/lustre/llite/llite_internal.h
> @@ -523,7 +523,11 @@ struct ll_sb_info {
>  	struct root_squash_info	  ll_squash;
>  	struct path		 ll_mnt;
>  
> -	__kernel_fsid_t		  ll_fsid;
> +	/* st_blksize returned by stat(2), when non-zero */
> +	unsigned int		 ll_stat_blksize;
> +
> +	__kernel_fsid_t		 ll_fsid;
> +
>  	struct kset		ll_kset;	/* sysfs object */
>  	struct completion	 ll_kobj_unregister;
>  };
> diff --git a/drivers/staging/lustre/lustre/llite/lproc_llite.c b/drivers/staging/lustre/lustre/llite/lproc_llite.c
> index f775b4f..d8ef090 100644
> --- a/drivers/staging/lustre/lustre/llite/lproc_llite.c
> +++ b/drivers/staging/lustre/lustre/llite/lproc_llite.c
> @@ -128,6 +128,38 @@ static ssize_t blocksize_show(struct kobject *kobj, struct attribute *attr,
>  }
>  LUSTRE_RO_ATTR(blocksize);
>  
> +static ssize_t stat_blocksize_show(struct kobject *kobj, struct attribute *attr,
> +				   char *buf)
> +{
> +	struct ll_sb_info *sbi = container_of(kobj, struct ll_sb_info,
> +					      ll_kset.kobj);
> +
> +	return sprintf(buf, "%u\n", sbi->ll_stat_blksize);
> +}
> +
> +static ssize_t stat_blocksize_store(struct kobject *kobj,
> +				    struct attribute *attr,
> +				    const char *buffer,
> +				    size_t count)
> +{
> +	struct ll_sb_info *sbi = container_of(kobj, struct ll_sb_info,
> +					      ll_kset.kobj);
> +	unsigned int val;
> +	int rc;
> +
> +	rc = kstrtouint(buffer, 10, &val);
> +	if (rc)
> +		return rc;
> +
> +	if (val != 0 && (val < PAGE_SIZE || (val & (val - 1))) != 0)
> +		return -ERANGE;
> +
> +	sbi->ll_stat_blksize = val;
> +
> +	return count;
> +}
> +LUSTRE_RW_ATTR(stat_blocksize);
> +
>  static ssize_t kbytestotal_show(struct kobject *kobj, struct attribute *attr,
>  				char *buf)
>  {
> @@ -1123,6 +1155,7 @@ static ssize_t ll_nosquash_nids_seq_write(struct file *file,
>  
>  static struct attribute *llite_attrs[] = {
>  	&lustre_attr_blocksize.attr,
> +	&lustre_attr_stat_blocksize.attr,
>  	&lustre_attr_kbytestotal.attr,
>  	&lustre_attr_kbytesfree.attr,
>  	&lustre_attr_kbytesavail.attr,
> -- 
> 1.8.3.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180924/af54ade6/attachment.sig>

[lustre-devel] [PATCH 21/30] lustre: ldlm: use static initializer macros where possible

[NeilBrown]

On Mon, Sep 17 2018, James Simmons wrote:

> From: "John L. Hammond" <jhammond@whamcloud.com>
>
> In lustre/ldlm/ replace module load time initialization of several
> mutexes with static initialization using the kernel provided macros.
>
> Signed-off-by: John L. Hammond <jhammond@whamcloud.com>
> WC-bug-id: https://jira.whamcloud.com/browse/LU-9010
> Reviewed-on: https://review.whamcloud.com/24824
> Reviewed-by: Dmitry Eremin <dmitry.eremin@intel.com>
> Reviewed-by: James Simmons <uja.ornl@yahoo.com>
> Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
> Signed-off-by: James Simmons <jsimmons@infradead.org>
> ---
>  drivers/staging/lustre/lustre/include/lustre_dlm.h | 3 +--
>  drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c    | 6 +-----
>  drivers/staging/lustre/lustre/ldlm/ldlm_resource.c | 4 ++--
>  3 files changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/include/lustre_dlm.h b/drivers/staging/lustre/lustre/include/lustre_dlm.h
> index a68c7a4..e2bbcaa 100644
> --- a/drivers/staging/lustre/lustre/include/lustre_dlm.h
> +++ b/drivers/staging/lustre/lustre/include/lustre_dlm.h
> @@ -742,8 +742,7 @@ struct ldlm_lock {
>  	 * The lists this could be linked into are:
>  	 * waiting_locks_list (protected by waiting_locks_spinlock),
>  	 * then if the lock timed out, it is moved to
> -	 * expired_lock_thread.elt_expired_locks for further processing.
> -	 * Protected by elt_lock.
> +	 * expired_lock_list for further processing.
>  	 */
>  	struct list_head		l_pending_chain;
>  
> diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
> index a8de3d9..986c378 100644
> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
> @@ -53,7 +53,7 @@
>  module_param(ldlm_cpts, charp, 0444);
>  MODULE_PARM_DESC(ldlm_cpts, "CPU partitions ldlm threads should run on");
>  
> -static struct mutex	ldlm_ref_mutex;
> +static DEFINE_MUTEX(ldlm_ref_mutex);
>  static int ldlm_refcount;
>  
>  static struct kobject *ldlm_kobj;
> @@ -69,10 +69,6 @@ struct ldlm_cb_async_args {
>  
>  static struct ldlm_state *ldlm_state;
>  
> -#define ELT_STOPPED   0
> -#define ELT_READY     1
> -#define ELT_TERMINATE 2
> -

Arg... Why do people do this???
One patch - one change.
This change is irrelevant to this patch, so it just makes it harder
to review.

Grumble.

NeilBrown


>  struct ldlm_bl_pool {
>  	spinlock_t		blp_lock;
>  
> diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c b/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c
> index 1907a5a..bd5622d 100644
> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c
> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c
> @@ -49,10 +49,10 @@
>  int ldlm_srv_namespace_nr;
>  int ldlm_cli_namespace_nr;
>  
> -struct mutex ldlm_srv_namespace_lock;
> +DEFINE_MUTEX(ldlm_srv_namespace_lock);
>  LIST_HEAD(ldlm_srv_namespace_list);
>  
> -struct mutex ldlm_cli_namespace_lock;
> +DEFINE_MUTEX(ldlm_cli_namespace_lock);
>  /* Client Namespaces that have active resources in them.
>   * Once all resources go away, ldlm_poold moves such namespaces to the
>   * inactive list
> -- 
> 1.8.3.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180924/1956aaa2/attachment.sig>

[lustre-devel] [PATCH 24/30] lustre: llite: rcu-walk check should not depend on statahead

[NeilBrown]

On Mon, Sep 17 2018, James Simmons wrote:

> From: Steve Guminski <stephenx.guminski@intel.com>
>
> Moves the check for the LOOKUP_RCU flag, so that it does not depend
> on the statahead setting.  The caller is now informed if rcu-walk
> was requested but the filesystem does not support it, regardless
> of whether statahead is enabled or disabled.

Nope, this is wrong.

The filesystem only returns -ECHILD if it couldn't complete the request
because it would have to block.
If statahead is disabled, then it can complete the request immediately,
doesn't need to block, and so doesn't need to return -ECHILD.

Patch deleted.

Thanks,
NeilBrown

>
> Signed-off-by: Steve Guminski <stephenx.guminski@intel.com>
> WC-bug-id: https://jira.whamcloud.com/browse/LU-8891
> Reviewed-on: https://review.whamcloud.com/24195
> Reviewed-by: John L. Hammond <jhammond@whamcloud.com>
> Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
> Reviewed-by: Oleg Drokin <green@whamcloud.com>
> Signed-off-by: James Simmons <jsimmons@infradead.org>
> ---
>  drivers/staging/lustre/lustre/llite/dcache.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/llite/dcache.c b/drivers/staging/lustre/lustre/llite/dcache.c
> index 11b82c63..ee1ba16 100644
> --- a/drivers/staging/lustre/lustre/llite/dcache.c
> +++ b/drivers/staging/lustre/lustre/llite/dcache.c
> @@ -270,13 +270,12 @@ static int ll_revalidate_dentry(struct dentry *dentry,
>  	if (lookup_flags & LOOKUP_REVAL)
>  		return 0;
>  
> -	if (!dentry_may_statahead(dir, dentry))
> -		return 1;
> -
>  	if (lookup_flags & LOOKUP_RCU)
>  		return -ECHILD;
>  
> -	ll_statahead(dir, &dentry, !d_inode(dentry));
> +	if (dentry_may_statahead(dir, dentry))
> +		ll_statahead(dir, &dentry, !d_inode(dentry));
> +
>  	return 1;
>  }
>  
> -- 
> 1.8.3.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180924/eeef3d51/attachment.sig>

[lustre-devel] [PATCH 11/30] lustre: mdc: allow setting readdir RPC size parameter

[James Simmons]

> On Sep 18, 2018, at 09:14, NeilBrown <neilb@suse.com> wrote:
> > 
> > On Mon, Sep 17 2018, James Simmons wrote:
> > 
> >> From: Andreas Dilger <adilger@whamcloud.com>
> >> 
> >> diff --git a/drivers/staging/lustre/lustre/include/lustre_net.h b/drivers/staging/lustre/lustre/include/lustre_net.h
> >> index 2dbd208..cf630db 100644
> >> --- a/drivers/staging/lustre/lustre/include/lustre_net.h
> >> +++ b/drivers/staging/lustre/lustre/include/lustre_net.h
> >> @@ -104,15 +104,15 @@
> >>  * currently supported maximum between peers at connect via ocd_brw_size.
> >>  */
> >> #define PTLRPC_MAX_BRW_BITS	(LNET_MTU_BITS + PTLRPC_BULK_OPS_BITS)
> >> -#define PTLRPC_MAX_BRW_SIZE	(1 << PTLRPC_MAX_BRW_BITS)
> >> +#define PTLRPC_MAX_BRW_SIZE	BIT(PTLRPC_MAX_BRW_BITS)
> >> #define PTLRPC_MAX_BRW_PAGES	(PTLRPC_MAX_BRW_SIZE >> PAGE_SHIFT)
> >> 
> >> -#define ONE_MB_BRW_SIZE		(1 << LNET_MTU_BITS)
> >> -#define MD_MAX_BRW_SIZE		(1 << LNET_MTU_BITS)
> >> +#define ONE_MB_BRW_SIZE		BIT(LNET_MTU_BITS)
> >> +#define MD_MAX_BRW_SIZE		BIT(LNET_MTU_BITS)
> >> #define MD_MAX_BRW_PAGES	(MD_MAX_BRW_SIZE >> PAGE_SHIFT)
> >> #define DT_MAX_BRW_SIZE		PTLRPC_MAX_BRW_SIZE
> >> #define DT_MAX_BRW_PAGES	(DT_MAX_BRW_SIZE >> PAGE_SHIFT)
> >> -#define OFD_MAX_BRW_SIZE	(1 << LNET_MTU_BITS)
> >> +#define OFD_MAX_BRW_SIZE	BIT(LNET_MTU_BITS)
> > 
> > Argg!!  No!!  Names are important.
> > "SIZE" means the value is a size, a number.  The bit-representation is
> > largely irrelevant, it is the number that is important.
> > BIT(x) returns a single bit - lots of zeros and just one '1' bit.  This
> > is not a number, it is a bit pattern.
> > 
> > So settings FOO_SIZE to BIT(bar) is wrong.  It is a type error.  It uses
> > a bit pattern when a number is expected.  The C compiler won't notice, but I will.
> > 
> > When I apply this (which probably won't be until next week), I'll just
> > remove this section of the patch.
> 
> Just to confirm, my original patch didn't have these BIT() macros in it,
> and I agree with your statements, so I'm fine with you removing them again.

That was my attempting to handle a checkpatch complaint with this patch. I 
do see your point in this. Still I like to keep the checkpatch numbers 
down to to remove a potential barrier to the acceptance into the fs tree.
We could replace (1 << LNET_MTU_BITS) with LNET_MTU which is the same 
value. I will latter post a separate patch to fix those checkpatch issues
up. 
 
> >> diff --git a/drivers/staging/lustre/lustre/llite/lcommon_cl.c b/drivers/staging/lustre/lustre/llite/lcommon_cl.c
> >> index 6c9fe49..d3b0445 100644
> >> --- a/drivers/staging/lustre/lustre/llite/lcommon_cl.c
> >> +++ b/drivers/staging/lustre/lustre/llite/lcommon_cl.c
> >> @@ -267,27 +267,22 @@ void cl_inode_fini(struct inode *inode)
> >> /**
> >>  * build inode number from passed @fid
> >>  */
> >> -__u64 cl_fid_build_ino(const struct lu_fid *fid, int api32)
> >> +u64 cl_fid_build_ino(const struct lu_fid *fid, int api32)
> >> {
> >> 	if (BITS_PER_LONG == 32 || api32)
> >> 		return fid_flatten32(fid);
> >> -	else
> >> -		return fid_flatten(fid);
> >> +
> >> +	return fid_flatten(fid);
> > 
> > I preferred that as it was - it makes the either/or nature more obvious.
> 
> Kernel style generally recommends no "else" after a return, and checkpatch.pl will complain in this case.
> 
> Cheers, Andreas
> ---
> Andreas Dilger
> Principal Lustre Architect
> Whamcloud
> 
> 
> 
> 
> 
> 
> 
> 

[lustre-devel] [PATCH 08/30] lustre: mgc: Remove unnecessary checks for config_log_put()

[James Simmons]

> > From: Steve Guminski <stephenx.guminski@intel.com>
> >
> > Make config_log_put() check if its parameter is NULL, which makes
> > it is unnecessary to perform the check prior to calling it.
> > This patch removes the redundant checks.
> >
> > Signed-off-by: Steve Guminski <stephenx.guminski@intel.com>
> > WC-bug-id: https://jira.whamcloud.com/browse/LU-9152
> > Reviewed-on: https://review.whamcloud.com/25854
> > Reviewed-by: Bob Glossman <bob.glossman@intel.com>
> > Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
> > Reviewed-by: James Simmons <uja.ornl@yahoo.com>
> > Signed-off-by: James Simmons <jsimmons@infradead.org>
> > ---
> >  drivers/staging/lustre/lustre/mgc/mgc_request.c | 36 +++++++++++--------------
> >  1 file changed, 16 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
> > index 4552cc5..e6f8d9e 100644
> > --- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
> > +++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
> > @@ -131,6 +131,9 @@ static int config_log_get(struct config_llog_data *cld)
> >   */
> >  static void config_log_put(struct config_llog_data *cld)
> >  {
> > +	if (unlikely(!cld))
> > +		return;
> > +
> 
> I've removed the "unlikely()" call.
> 
> Partly because use of likely/unlikely is discouraged where the
> difference cannot be measured (or clearly justified some other way),
> and partly because .....

I kept it in since it was in the original patch. Personally I'm not a fan
either. The only time I would use it is in a case where code is executed
100k+ times a second and even then I question if you really get any boost
with modern processors. 
 
> > @@ -387,6 +387,9 @@ struct config_llog_data *do_config_log_add(struct obd_device *obd,
> >  
> >  static inline void config_mark_cld_stop(struct config_llog_data *cld)
> >  {
> > +	if (!cld)
> > +		return;
> > +
> 
> .... there is no "unlikely()" here, and I like consistency.
> 
> Thanks,
> NeilBrown
> 
> 
> >  	mutex_lock(&cld->cld_lock);
> >  	spin_lock(&config_list_lock);
> >  	cld->cld_stopping = 1;
> > @@ -436,18 +439,13 @@ static int config_log_end(char *logname, struct config_llog_instance *cfg)
> >  	cld->cld_sptlrpc = NULL;
> >  	mutex_unlock(&cld->cld_lock);
> >  
> > -	if (cld_recover) {
> > -		config_mark_cld_stop(cld_recover);
> > -		config_log_put(cld_recover);
> > -	}
> > +	config_mark_cld_stop(cld_recover);
> > +	config_log_put(cld_recover);
> >  
> > -	if (cld_params) {
> > -		config_mark_cld_stop(cld_params);
> > -		config_log_put(cld_params);
> > -	}
> > +	config_mark_cld_stop(cld_params);
> > +	config_log_put(cld_params);
> >  
> > -	if (cld_sptlrpc)
> > -		config_log_put(cld_sptlrpc);
> > +	config_log_put(cld_sptlrpc);
> >  
> >  	/* drop the ref from the find */
> >  	config_log_put(cld);
> > @@ -593,8 +591,7 @@ static int mgc_requeue_thread(void *data)
> >  			cld->cld_lostlock = 0;
> >  			spin_unlock(&config_list_lock);
> >  
> > -			if (cld_prev)
> > -				config_log_put(cld_prev);
> > +			config_log_put(cld_prev);
> >  			cld_prev = cld;
> >  
> >  			if (likely(!(rq_state & RQ_STOP))) {
> > @@ -606,8 +603,7 @@ static int mgc_requeue_thread(void *data)
> >  			}
> >  		}
> >  		spin_unlock(&config_list_lock);
> > -		if (cld_prev)
> > -			config_log_put(cld_prev);
> > +		config_log_put(cld_prev);
> >  
> >  		/* Wait a bit to see if anyone else needs a requeue */
> >  		wait_event_idle(rq_waitq, rq_state & (RQ_NOW | RQ_STOP));
> > -- 
> > 1.8.3.1
> 

[lustre-devel] [PATCH 17/30] lustre: llite: protect ll_dentry_data modification

[James Simmons]

> 
> > From: Andriy Skulysh <c17819@cray.com>
> >
> > The ll_dentry_data bitfields modification should be protected by
> > a spinlock.
> >
> > Signed-off-by: Andriy Skulysh <c17819@cray.com>
> > Signed-off-by: Lai Siyao <lai.siyao@whamcloud.com>
> > WC-bug-id: https://jira.whamcloud.com/browse/LU-9241
> > Seagate-bug-id: MRP-4257
> > Reviewed-on: https://review.whamcloud.com/26109
> > Reviewed-by: Niu Yawei <yawei.niu@intel.com>
> > Reviewed-by: Bobi Jam <bobijam@hotmail.com>
> > Reviewed-by: Oleg Drokin <green@whamcloud.com>
> > Signed-off-by: James Simmons <jsimmons@infradead.org>
> > ---
> >  drivers/staging/lustre/lustre/llite/llite_nfs.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/lustre/lustre/llite/llite_nfs.c b/drivers/staging/lustre/lustre/llite/llite_nfs.c
> > index c66072a..5e91e83 100644
> > --- a/drivers/staging/lustre/lustre/llite/llite_nfs.c
> > +++ b/drivers/staging/lustre/lustre/llite/llite_nfs.c
> > @@ -171,8 +171,9 @@ struct lustre_nfs_fid {
> >  	 * we came from NFS and so opencache needs to be
> >  	 * enabled for this one
> >  	 */
> > +	spin_lock(&result->d_lock);
> >  	ll_d2d(result)->lld_nfs_dentry = 1;
> > -
> > +	spin_unlock(&result->d_lock);
> >  	return result;
> >  }
> 
> This is a bit weird....
> I agree that having a spinlock is needed as the compiler does an R/M/W
> to update the bitfield, and that could race with updats to lld_invalid
> (and I think that is a good arguement to avoid bitfields in shared structures).
> 
> However lld_nfs_dentry is never used.  The only use was removed by
> 
> Commit: c1b66fccf986 ("staging: lustre: fid: do open-by-fid by default")
> 
> The corresponding commit upstream is
> 
> Commit: cb85c0364fd8 ("LU-3544 fid: do open-by-fid by default")
> 
> and that commit doesn't remove the use of lld_nfs_dentry.  Maybe because
> lld_nfs_dentry didn't exist then - it wasn't added until later
> by 65d0add6057b138.
> 
> So do we need to bring lld_nfs_dentry back?

Ouch. That got removed by mistake by me ;-( Yes it should come back. 
I will create a patch to fix that. My bad.

[lustre-devel] [PATCH 24/30] lustre: llite: rcu-walk check should not depend on statahead

[James Simmons]

> On Mon, Sep 17 2018, James Simmons wrote:
> 
> > From: Steve Guminski <stephenx.guminski@intel.com>
> >
> > Moves the check for the LOOKUP_RCU flag, so that it does not depend
> > on the statahead setting.  The caller is now informed if rcu-walk
> > was requested but the filesystem does not support it, regardless
> > of whether statahead is enabled or disabled.
> 
> Nope, this is wrong.
> 
> The filesystem only returns -ECHILD if it couldn't complete the request
> because it would have to block.
> If statahead is disabled, then it can complete the request immediately,
> doesn't need to block, and so doesn't need to return -ECHILD.
> 
> Patch deleted.

Do this patch need to be reverted in the OpenSFS branch or can it be
ignored?

> > Signed-off-by: Steve Guminski <stephenx.guminski@intel.com>
> > WC-bug-id: https://jira.whamcloud.com/browse/LU-8891
> > Reviewed-on: https://review.whamcloud.com/24195
> > Reviewed-by: John L. Hammond <jhammond@whamcloud.com>
> > Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
> > Reviewed-by: Oleg Drokin <green@whamcloud.com>
> > Signed-off-by: James Simmons <jsimmons@infradead.org>
> > ---
> >  drivers/staging/lustre/lustre/llite/dcache.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/lustre/lustre/llite/dcache.c b/drivers/staging/lustre/lustre/llite/dcache.c
> > index 11b82c63..ee1ba16 100644
> > --- a/drivers/staging/lustre/lustre/llite/dcache.c
> > +++ b/drivers/staging/lustre/lustre/llite/dcache.c
> > @@ -270,13 +270,12 @@ static int ll_revalidate_dentry(struct dentry *dentry,
> >  	if (lookup_flags & LOOKUP_REVAL)
> >  		return 0;
> >  
> > -	if (!dentry_may_statahead(dir, dentry))
> > -		return 1;
> > -
> >  	if (lookup_flags & LOOKUP_RCU)
> >  		return -ECHILD;
> >  
> > -	ll_statahead(dir, &dentry, !d_inode(dentry));
> > +	if (dentry_may_statahead(dir, dentry))
> > +		ll_statahead(dir, &dentry, !d_inode(dentry));
> > +
> >  	return 1;
> >  }
> >  
> > -- 
> > 1.8.3.1
> 

[lustre-devel] [PATCH 24/30] lustre: llite: rcu-walk check should not depend on statahead

[NeilBrown]

On Sat, Sep 29 2018, James Simmons wrote:

>> On Mon, Sep 17 2018, James Simmons wrote:
>> 
>> > From: Steve Guminski <stephenx.guminski@intel.com>
>> >
>> > Moves the check for the LOOKUP_RCU flag, so that it does not depend
>> > on the statahead setting.  The caller is now informed if rcu-walk
>> > was requested but the filesystem does not support it, regardless
>> > of whether statahead is enabled or disabled.
>> 
>> Nope, this is wrong.
>> 
>> The filesystem only returns -ECHILD if it couldn't complete the request
>> because it would have to block.
>> If statahead is disabled, then it can complete the request immediately,
>> doesn't need to block, and so doesn't need to return -ECHILD.
>> 
>> Patch deleted.
>
> Do this patch need to be reverted in the OpenSFS branch or can it be
> ignored?

It can be ignored.
The justification for the patch (and it is definitely nice to see a
clearly stated justification!!) was based on a misunderstanding, and as
wrong.
The code itself introduces a small performance hit when statahead is not
being used.  I can't really say if it would be noticeable or not, but
you could only notice it on a many CPU machine where lots of the
filesystem was cached locally, and there was no need to go to the server
to service lots of lookups.

NeilBrown


>
>> > Signed-off-by: Steve Guminski <stephenx.guminski@intel.com>
>> > WC-bug-id: https://jira.whamcloud.com/browse/LU-8891
>> > Reviewed-on: https://review.whamcloud.com/24195
>> > Reviewed-by: John L. Hammond <jhammond@whamcloud.com>
>> > Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
>> > Reviewed-by: Oleg Drokin <green@whamcloud.com>
>> > Signed-off-by: James Simmons <jsimmons@infradead.org>
>> > ---
>> >  drivers/staging/lustre/lustre/llite/dcache.c | 7 +++----
>> >  1 file changed, 3 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/staging/lustre/lustre/llite/dcache.c b/drivers/staging/lustre/lustre/llite/dcache.c
>> > index 11b82c63..ee1ba16 100644
>> > --- a/drivers/staging/lustre/lustre/llite/dcache.c
>> > +++ b/drivers/staging/lustre/lustre/llite/dcache.c
>> > @@ -270,13 +270,12 @@ static int ll_revalidate_dentry(struct dentry *dentry,
>> >  	if (lookup_flags & LOOKUP_REVAL)
>> >  		return 0;
>> >  
>> > -	if (!dentry_may_statahead(dir, dentry))
>> > -		return 1;
>> > -
>> >  	if (lookup_flags & LOOKUP_RCU)
>> >  		return -ECHILD;
>> >  
>> > -	ll_statahead(dir, &dentry, !d_inode(dentry));
>> > +	if (dentry_may_statahead(dir, dentry))
>> > +		ll_statahead(dir, &dentry, !d_inode(dentry));
>> > +
>> >  	return 1;
>> >  }
>> >  
>> > -- 
>> > 1.8.3.1
>> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20181002/cf2a0e2e/attachment.sig>