All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 04/16] nfsd: add nfsd/clients directory
From: J. Bruce Fields @ 2019-06-20 14:51 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields
In-Reply-To: <1561042275-12723-1-git-send-email-bfields@redhat.com>

From: "J. Bruce Fields" <bfields@redhat.com>

I plan to expose some information about nfsv4 clients here.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/netns.h     |   2 +
 fs/nfsd/nfs4state.c |  23 ++++++----
 fs/nfsd/nfsctl.c    | 103 +++++++++++++++++++++++++++++++++++++++++++-
 fs/nfsd/nfsd.h      |   9 ++++
 fs/nfsd/state.h     |   6 ++-
 5 files changed, 133 insertions(+), 10 deletions(-)

diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
index e7890e87d30b..cee843e8e440 100644
--- a/fs/nfsd/netns.h
+++ b/fs/nfsd/netns.h
@@ -58,6 +58,8 @@ struct nfsd_net {
 	/* internal mount of the "nfsd" pseudofilesystem: */
 	struct vfsmount *nfsd_mnt;
 
+	struct dentry *nfsd_client_dir;
+
 	/*
 	 * reclaim_str_hashtbl[] holds known client info from previous reset/reboot
 	 * used in reboot/reset lease grace period processing
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d2ea41add566..cf081a8d6b6b 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1881,20 +1881,19 @@ static struct nfs4_client *alloc_client(struct xdr_netobj name)
 
 static void __free_client(struct kref *k)
 {
-	struct nfs4_client *clp = container_of(k, struct nfs4_client, cl_ref);
+	struct nfsdfs_client *c = container_of(k, struct nfsdfs_client, cl_ref);
+	struct nfs4_client *clp = container_of(c, struct nfs4_client, cl_nfsdfs);
 
 	free_svc_cred(&clp->cl_cred);
 	kfree(clp->cl_ownerstr_hashtbl);
 	kfree(clp->cl_name.data);
 	idr_destroy(&clp->cl_stateids);
-	if (clp->cl_nfsd_dentry)
-		nfsd_client_rmdir(clp->cl_nfsd_dentry);
 	kmem_cache_free(client_slab, clp);
 }
 
 void drop_client(struct nfs4_client *clp)
 {
-	kref_put(&clp->cl_ref, __free_client);
+	kref_put(&clp->cl_nfsdfs.cl_ref, __free_client);
 }
 
 static void
@@ -1909,6 +1908,8 @@ free_client(struct nfs4_client *clp)
 		free_session(ses);
 	}
 	rpc_destroy_wait_queue(&clp->cl_cb_waitq);
+	if (clp->cl_nfsd_dentry)
+		nfsd_client_rmdir(clp->cl_nfsd_dentry);
 	drop_client(clp);
 }
 
@@ -2220,6 +2221,7 @@ static struct nfs4_client *create_client(struct xdr_netobj name,
 	struct sockaddr *sa = svc_addr(rqstp);
 	int ret;
 	struct net *net = SVC_NET(rqstp);
+	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
 
 	clp = alloc_client(name);
 	if (clp == NULL)
@@ -2230,8 +2232,8 @@ static struct nfs4_client *create_client(struct xdr_netobj name,
 		free_client(clp);
 		return NULL;
 	}
-
-	kref_init(&clp->cl_ref);
+	gen_clid(clp, nn);
+	kref_init(&clp->cl_nfsdfs.cl_ref);
 	nfsd4_init_cb(&clp->cl_cb_null, clp, NULL, NFSPROC4_CLNT_CB_NULL);
 	clp->cl_time = get_seconds();
 	clear_bit(0, &clp->cl_cb_slot_busy);
@@ -2239,6 +2241,12 @@ static struct nfs4_client *create_client(struct xdr_netobj name,
 	rpc_copy_addr((struct sockaddr *) &clp->cl_addr, sa);
 	clp->cl_cb_session = NULL;
 	clp->net = net;
+	clp->cl_nfsd_dentry = nfsd_client_mkdir(nn, &clp->cl_nfsdfs,
+						clp->cl_clientid.cl_id);
+	if (!clp->cl_nfsd_dentry) {
+		free_client(clp);
+		return NULL;
+	}
 	return clp;
 }
 
@@ -2683,7 +2691,6 @@ nfsd4_exchange_id(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	new->cl_spo_must_allow.u.words[0] = exid->spo_must_allow[0];
 	new->cl_spo_must_allow.u.words[1] = exid->spo_must_allow[1];
 
-	gen_clid(new, nn);
 	add_to_unconfirmed(new);
 	swap(new, conf);
 out_copy:
@@ -3427,7 +3434,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		copy_clid(new, conf);
 		gen_confirm(new, nn);
 	} else /* case 4 (new client) or cases 2, 3 (client reboot): */
-		gen_clid(new, nn);
+		;
 	new->cl_minorversion = 0;
 	gen_callback(new, setclid, rqstp);
 	add_to_unconfirmed(new);
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index ab1fb81b7f5e..638a25648dcb 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -15,6 +15,7 @@
 #include <linux/sunrpc/gss_krb5_enctypes.h>
 #include <linux/sunrpc/rpc_pipe_fs.h>
 #include <linux/module.h>
+#include <linux/fsnotify.h>
 
 #include "idmap.h"
 #include "nfsd.h"
@@ -52,6 +53,7 @@ enum {
 	NFSD_RecoveryDir,
 	NFSD_V4EndGrace,
 #endif
+	NFSD_MaxReserved
 };
 
 /*
@@ -1146,8 +1148,99 @@ static ssize_t write_v4_end_grace(struct file *file, char *buf, size_t size)
  *	populating the filesystem.
  */
 
+/* Basically copying rpc_get_inode. */
+static struct inode *nfsd_get_inode(struct super_block *sb, umode_t mode)
+{
+	struct inode *inode = new_inode(sb);
+	if (!inode)
+		return NULL;
+	/* Following advice from simple_fill_super documentation: */
+	inode->i_ino = iunique(sb, NFSD_MaxReserved);
+	inode->i_mode = mode;
+	inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode);
+	switch (mode & S_IFMT) {
+	case S_IFDIR:
+		inode->i_fop = &simple_dir_operations;
+		inode->i_op = &simple_dir_inode_operations;
+		inc_nlink(inode);
+	default:
+		break;
+	}
+	return inode;
+}
+
+static int __nfsd_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
+{
+	struct inode *inode;
+
+	inode = nfsd_get_inode(dir->i_sb, mode);
+	if (!inode)
+		return -ENOMEM;
+	d_add(dentry, inode);
+	inc_nlink(dir);
+	fsnotify_mkdir(dir, dentry);
+	return 0;
+}
+
+static struct dentry *nfsd_mkdir(struct dentry *parent, struct nfsdfs_client *ncl, char *name)
+{
+	struct inode *dir = parent->d_inode;
+	struct dentry *dentry;
+	int ret = -ENOMEM;
+
+	inode_lock(dir);
+	dentry = d_alloc_name(parent, name);
+	if (!dentry)
+		goto out_err;
+	ret = __nfsd_mkdir(d_inode(parent), dentry, S_IFDIR | 0600);
+	if (ret)
+		goto out_err;
+	if (ncl) {
+		d_inode(dentry)->i_private = ncl;
+		kref_get(&ncl->cl_ref);
+	}
+out:
+	inode_unlock(dir);
+	return dentry;
+out_err:
+	dentry = ERR_PTR(ret);
+	goto out;
+}
+
+/* on success, returns positive number unique to that client. */
+struct dentry *nfsd_client_mkdir(struct nfsd_net *nn, struct nfsdfs_client *ncl, u32 id)
+{
+	char name[11];
+
+	sprintf(name, "%d", id++);
+
+	return nfsd_mkdir(nn->nfsd_client_dir, ncl, name);
+}
+
+/* Taken from __rpc_rmdir: */
+void nfsd_client_rmdir(struct dentry *dentry)
+{
+	struct inode *dir = d_inode(dentry->d_parent);
+	struct inode *inode = d_inode(dentry);
+	struct nfsdfs_client *ncl = inode->i_private;
+	int ret;
+
+	inode->i_private = NULL;
+	synchronize_rcu();
+	kref_put(&ncl->cl_ref, ncl->cl_release);
+	dget(dentry);
+	ret = simple_rmdir(dir, dentry);
+	WARN_ON_ONCE(ret);
+	d_delete(dentry);
+}
+
 static int nfsd_fill_super(struct super_block * sb, void * data, int silent)
 {
+	struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
+							nfsd_net_id);
+	struct dentry *dentry;
+	int ret;
+
 	static const struct tree_descr nfsd_files[] = {
 		[NFSD_List] = {"exports", &exports_nfsd_operations, S_IRUGO},
 		[NFSD_Export_features] = {"export_features",
@@ -1177,7 +1270,15 @@ static int nfsd_fill_super(struct super_block * sb, void * data, int silent)
 		/* last one */ {""}
 	};
 	get_net(sb->s_fs_info);
-	return simple_fill_super(sb, 0x6e667364, nfsd_files);
+	ret = simple_fill_super(sb, 0x6e667364, nfsd_files);
+	if (ret)
+		return ret;
+	dentry = nfsd_mkdir(sb->s_root, NULL, "clients");
+	if (IS_ERR(dentry))
+		return PTR_ERR(dentry);
+	nn->nfsd_client_dir = dentry;
+	return 0;
+
 }
 
 static struct dentry *nfsd_mount(struct file_system_type *fs_type,
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 24187b5dd638..85525dcbf77d 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -22,6 +22,7 @@
 
 #include <uapi/linux/nfsd/debug.h>
 
+#include "netns.h"
 #include "stats.h"
 #include "export.h"
 
@@ -86,6 +87,14 @@ int		nfsd_pool_stats_release(struct inode *, struct file *);
 
 void		nfsd_destroy(struct net *net);
 
+struct nfsdfs_client {
+	struct kref cl_ref;
+	void (*cl_release)(struct kref *kref);
+};
+
+struct dentry *nfsd_client_mkdir(struct nfsd_net *nn, struct nfsdfs_client *ncl, u32 id);
+void nfsd_client_rmdir(struct dentry *dentry);
+
 #if defined(CONFIG_NFSD_V2_ACL) || defined(CONFIG_NFSD_V3_ACL)
 #ifdef CONFIG_NFSD_V2_ACL
 extern const struct svc_version nfsd_acl_version2;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 8eacdbc50cd7..81852cbf6b0a 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -39,6 +39,7 @@
 #include <linux/refcount.h>
 #include <linux/sunrpc/svc_xprt.h>
 #include "nfsfh.h"
+#include "nfsd.h"
 
 typedef struct {
 	u32             cl_boot;
@@ -348,9 +349,12 @@ struct nfs4_client {
 	u32			cl_exchange_flags;
 	/* number of rpc's in progress over an associated session: */
 	atomic_t		cl_rpc_users;
-	struct kref		cl_ref;
+	struct nfsdfs_client	cl_nfsdfs;
 	struct nfs4_op_map      cl_spo_must_allow;
 
+	/* debugging info directory under nfsd/clients/ : */
+	struct dentry		*cl_nfsd_dentry;
+
 	/* for nfs41 callbacks */
 	/* We currently support a single back channel with a single slot */
 	unsigned long		cl_cb_slot_busy;
-- 
2.21.0


^ permalink raw reply related

* [PATCH 16/16] nfsd: decode implementation id
From: J. Bruce Fields @ 2019-06-20 14:51 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields
In-Reply-To: <1561042275-12723-1-git-send-email-bfields@redhat.com>

From: "J. Bruce Fields" <bfields@redhat.com>

Decode the implementation ID and display in nfsd/clients/#/info.  It may
be help identify the client.  It won't be used otherwise.

(When this went into the protocol, I thought the implementation ID would
be a slippery slope towards implementation-specific workarounds as with
the http user-agent.  But I guess I was wrong, the risk seems pretty low
now.)

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 30 ++++++++++++++++++++++++++++++
 fs/nfsd/nfs4xdr.c   | 21 +++++++++------------
 fs/nfsd/state.h     |  4 ++++
 fs/nfsd/xdr4.h      |  3 +++
 4 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 8f35e440ef14..4fcbb5d809a6 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1899,6 +1899,8 @@ static void __free_client(struct kref *k)
 	free_svc_cred(&clp->cl_cred);
 	kfree(clp->cl_ownerstr_hashtbl);
 	kfree(clp->cl_name.data);
+	kfree(clp->cl_nii_domain.data);
+	kfree(clp->cl_nii_name.data);
 	idr_destroy(&clp->cl_stateids);
 	kmem_cache_free(client_slab, clp);
 }
@@ -2261,6 +2263,15 @@ static int client_info_show(struct seq_file *m, void *v)
 	seq_printf(m, "name: ");
 	seq_quote_mem(m, clp->cl_name.data, clp->cl_name.len);
 	seq_printf(m, "\nminor version: %d\n", clp->cl_minorversion);
+	if (clp->cl_nii_domain.data) {
+		seq_printf(m, "Implementation domain: ");
+		seq_quote_mem(m, clp->cl_nii_domain.data,
+					clp->cl_nii_domain.len);
+		seq_printf(m, "\nImplementation name: ");
+		seq_quote_mem(m, clp->cl_nii_name.data, clp->cl_nii_name.len);
+		seq_printf(m, "\nImplementation time: [%ld, %ld]\n",
+			clp->cl_nii_time.tv_sec, clp->cl_nii_time.tv_nsec);
+	}
 	drop_client(clp);
 
 	return 0;
@@ -2901,6 +2912,22 @@ static bool client_has_state(struct nfs4_client *clp)
 		|| !list_empty(&clp->async_copies);
 }
 
+static __be32 copy_impl_id(struct nfs4_client *clp,
+				struct nfsd4_exchange_id *exid)
+{
+	if (!exid->nii_domain.data)
+		return 0;
+	xdr_netobj_dup(&clp->cl_nii_domain, &exid->nii_domain, GFP_KERNEL);
+	if (!clp->cl_nii_domain.data)
+		return nfserr_jukebox;
+	xdr_netobj_dup(&clp->cl_nii_name, &exid->nii_name, GFP_KERNEL);
+	if (!clp->cl_nii_name.data)
+		return nfserr_jukebox;
+	clp->cl_nii_time.tv_sec = exid->nii_time.tv_sec;
+	clp->cl_nii_time.tv_nsec = exid->nii_time.tv_nsec;
+	return 0;
+}
+
 __be32
 nfsd4_exchange_id(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		union nfsd4_op_u *u)
@@ -2927,6 +2954,9 @@ nfsd4_exchange_id(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	new = create_client(exid->clname, rqstp, &verf);
 	if (new == NULL)
 		return nfserr_jukebox;
+	status = copy_impl_id(new, exid);
+	if (status)
+		goto out_nolock;
 
 	switch (exid->spa_how) {
 	case SP4_MACH_CRED:
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 52c4f6daa649..3bb147822205 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1398,7 +1398,6 @@ nfsd4_decode_exchange_id(struct nfsd4_compoundargs *argp,
 		goto xdr_error;
 	}
 
-	/* Ignore Implementation ID */
 	READ_BUF(4);    /* nfs_impl_id4 array length */
 	dummy = be32_to_cpup(p++);
 
@@ -1406,21 +1405,19 @@ nfsd4_decode_exchange_id(struct nfsd4_compoundargs *argp,
 		goto xdr_error;
 
 	if (dummy == 1) {
-		/* nii_domain */
-		READ_BUF(4);
-		dummy = be32_to_cpup(p++);
-		READ_BUF(dummy);
-		p += XDR_QUADLEN(dummy);
+		status = nfsd4_decode_opaque(argp, &exid->nii_domain);
+		if (status)
+			goto xdr_error;
 
 		/* nii_name */
-		READ_BUF(4);
-		dummy = be32_to_cpup(p++);
-		READ_BUF(dummy);
-		p += XDR_QUADLEN(dummy);
+		status = nfsd4_decode_opaque(argp, &exid->nii_name);
+		if (status)
+			goto xdr_error;
 
 		/* nii_date */
-		READ_BUF(12);
-		p += 3;
+		status = nfsd4_decode_time(argp, &exid->nii_time);
+		if (status)
+			goto xdr_error;
 	}
 	DECODE_TAIL;
 }
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 81852cbf6b0a..8cb20cab012b 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -317,6 +317,10 @@ struct nfs4_client {
 	clientid_t		cl_clientid;	/* generated by server */
 	nfs4_verifier		cl_confirm;	/* generated by server */
 	u32			cl_minorversion;
+	/* NFSv4.1 client implementation id: */
+	struct xdr_netobj	cl_nii_domain;
+	struct xdr_netobj	cl_nii_name;
+	struct timespec		cl_nii_time;
 
 	/* for v4.0 and v4.1 callbacks: */
 	struct nfs4_cb_conn	cl_cb_conn;
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index feeb6d4bdffd..a5222fc9ea44 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -410,6 +410,9 @@ struct nfsd4_exchange_id {
 	int		spa_how;
 	u32             spo_must_enforce[3];
 	u32             spo_must_allow[3];
+	struct xdr_netobj nii_domain;
+	struct xdr_netobj nii_name;
+	struct timespec nii_time;
 };
 
 struct nfsd4_sequence {
-- 
2.21.0


^ permalink raw reply related

* [PATCH 11/16] nfsd: show lock and deleg stateids
From: J. Bruce Fields @ 2019-06-20 14:51 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields
In-Reply-To: <1561042275-12723-1-git-send-email-bfields@redhat.com>

From: "J. Bruce Fields" <bfields@redhat.com>

These entries are pretty minimal for now.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 59 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index ad9487afd3c5..ab4302cee2d2 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2338,12 +2338,66 @@ static int nfs4_show_open(struct seq_file *s, struct nfs4_stid *st)
 	seq_printf(s, ", ");
 	nfs4_show_owner(s, oo);
 	seq_printf(s, " }\n");
+	fput(file);
+
+	return 0;
+}
+
+static int nfs4_show_lock(struct seq_file *s, struct nfs4_stid *st)
+{
+	struct nfs4_ol_stateid *ols;
+	struct nfs4_file *nf;
+	struct file *file;
+	struct nfs4_stateowner *oo;
 
+	ols = openlockstateid(st);
+	oo = ols->st_stateowner;
+	nf = st->sc_file;
+	file = find_any_file(nf);
+
+	seq_printf(s, "- 0x%16phN: { type: lock, ", &st->sc_stateid);
+
+	/*
+	 * Note: a lock stateid isn't really the same thing as a lock,
+	 * it's the locking state held by one owner on a file, and there
+	 * may be multiple (or no) lock ranges associated with it.
+	 * (Same for the matter is true of open stateids.)
+	 */
+
+	nfs4_show_superblock(s, file);
+	/* XXX: open stateid? */
+	seq_printf(s, ", ");
+	nfs4_show_owner(s, oo);
+	seq_printf(s, " }\n");
 	fput(file);
 
 	return 0;
 }
 
+static int nfs4_show_deleg(struct seq_file *s, struct nfs4_stid *st)
+{
+	struct nfs4_delegation *ds;
+	struct nfs4_file *nf;
+	struct file *file;
+
+	ds = delegstateid(st);
+	nf = st->sc_file;
+	file = nf->fi_deleg_file;
+
+	seq_printf(s, "- 0x%16phN: { type: deleg, ", &st->sc_stateid);
+
+	/* Kinda dead code as long as we only support read delegs: */
+	seq_printf(s, "access: %s, ",
+		ds->dl_type == NFS4_OPEN_DELEGATE_READ ? "r" : "w");
+
+	/* XXX: lease time, whether it's being recalled. */
+
+	nfs4_show_superblock(s, file);
+	seq_printf(s, " }\n");
+
+	return 0;
+}
+
 static int states_show(struct seq_file *s, void *v)
 {
 	struct nfs4_stid *st = v;
@@ -2351,9 +2405,14 @@ static int states_show(struct seq_file *s, void *v)
 	switch (st->sc_type) {
 	case NFS4_OPEN_STID:
 		return nfs4_show_open(s, st);
+	case NFS4_LOCK_STID:
+		return nfs4_show_lock(s, st);
+	case NFS4_DELEG_STID:
+		return nfs4_show_deleg(s, st);
 	default:
 		return 0; /* XXX: or SEQ_SKIP? */
 	}
+	/* XXX: copy stateids? */
 }
 
 static struct seq_operations states_seq_ops = {
-- 
2.21.0


^ permalink raw reply related

* [PATCH 10/16] nfsd4: add file to display list of client's opens
From: J. Bruce Fields @ 2019-06-20 14:51 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields
In-Reply-To: <1561042275-12723-1-git-send-email-bfields@redhat.com>

From: "J. Bruce Fields" <bfields@redhat.com>

Add a nfsd/clients/#/opens file to list some information about all the
opens held by the given client, including open modes, device numbers,
inode numbers, and open owners.

Open owners are totally opaque but seem to sometimes have some useful
ascii strings included, so passing through printable ascii characters
and escaping the rest seems useful while still being machine-readable.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 149 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 147 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 430cea90a715..ad9487afd3c5 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -695,7 +695,8 @@ struct nfs4_stid *nfs4_alloc_stid(struct nfs4_client *cl, struct kmem_cache *sla
 
 	idr_preload(GFP_KERNEL);
 	spin_lock(&cl->cl_lock);
-	new_id = idr_alloc_cyclic(&cl->cl_stateids, stid, 0, 0, GFP_NOWAIT);
+	/* Reserving 0 for start of file in nfsdfs "states" file: */
+	new_id = idr_alloc_cyclic(&cl->cl_stateids, stid, 1, 0, GFP_NOWAIT);
 	spin_unlock(&cl->cl_lock);
 	idr_preload_end();
 	if (new_id < 0)
@@ -2256,9 +2257,153 @@ static const struct file_operations client_info_fops = {
 	.release	= single_release,
 };
 
+static void *states_start(struct seq_file *s, loff_t *pos)
+	__acquires(&clp->cl_lock)
+{
+	struct nfs4_client *clp = s->private;
+	unsigned long id = *pos;
+	void *ret;
+
+	spin_lock(&clp->cl_lock);
+	ret = idr_get_next_ul(&clp->cl_stateids, &id);
+	*pos = id;
+	return ret;
+}
+
+static void *states_next(struct seq_file *s, void *v, loff_t *pos)
+{
+	struct nfs4_client *clp = s->private;
+	unsigned long id = *pos;
+	void *ret;
+
+	id = *pos;
+	id++;
+	ret = idr_get_next_ul(&clp->cl_stateids, &id);
+	*pos = id;
+	return ret;
+}
+
+static void states_stop(struct seq_file *s, void *v)
+	__releases(&clp->cl_lock)
+{
+	struct nfs4_client *clp = s->private;
+
+	spin_unlock(&clp->cl_lock);
+}
+
+static void nfs4_show_superblock(struct seq_file *s, struct file *f)
+{
+	struct inode *inode = file_inode(f);
+
+	seq_printf(s, "superblock: \"%02x:%02x:%ld\"",
+					MAJOR(inode->i_sb->s_dev),
+					 MINOR(inode->i_sb->s_dev),
+					 inode->i_ino);
+}
+
+static void nfs4_show_owner(struct seq_file *s, struct nfs4_stateowner *oo)
+{
+	seq_printf(s, "owner: ");
+	seq_quote_mem(s, oo->so_owner.data, oo->so_owner.len);
+}
+
+static int nfs4_show_open(struct seq_file *s, struct nfs4_stid *st)
+{
+	struct nfs4_ol_stateid *ols;
+	struct nfs4_file *nf;
+	struct file *file;
+	struct nfs4_stateowner *oo;
+	unsigned int access, deny;
+
+	if (st->sc_type != NFS4_OPEN_STID && st->sc_type != NFS4_LOCK_STID)
+		return 0; /* XXX: or SEQ_SKIP? */
+	ols = openlockstateid(st);
+	oo = ols->st_stateowner;
+	nf = st->sc_file;
+	file = find_any_file(nf);
+
+	seq_printf(s, "- 0x%16phN: { type: open, ", &st->sc_stateid);
+
+	access = bmap_to_share_mode(ols->st_access_bmap);
+	deny   = bmap_to_share_mode(ols->st_deny_bmap);
+
+	seq_printf(s, "access: \%s\%s, ",
+		access & NFS4_SHARE_ACCESS_READ ? "r" : "-",
+		access & NFS4_SHARE_ACCESS_WRITE ? "w" : "-");
+	seq_printf(s, "deny: \%s\%s, ",
+		deny & NFS4_SHARE_ACCESS_READ ? "r" : "-",
+		deny & NFS4_SHARE_ACCESS_WRITE ? "w" : "-");
+
+	nfs4_show_superblock(s, file);
+	seq_printf(s, ", ");
+	nfs4_show_owner(s, oo);
+	seq_printf(s, " }\n");
+
+	fput(file);
+
+	return 0;
+}
+
+static int states_show(struct seq_file *s, void *v)
+{
+	struct nfs4_stid *st = v;
+
+	switch (st->sc_type) {
+	case NFS4_OPEN_STID:
+		return nfs4_show_open(s, st);
+	default:
+		return 0; /* XXX: or SEQ_SKIP? */
+	}
+}
+
+static struct seq_operations states_seq_ops = {
+	.start = states_start,
+	.next = states_next,
+	.stop = states_stop,
+	.show = states_show
+};
+
+static int client_states_open(struct inode *inode, struct file *file)
+{
+	struct nfsdfs_client *nc;
+	struct seq_file *s;
+	struct nfs4_client *clp;
+	int ret;
+
+	nc = get_nfsdfs_client(inode);
+	if (!nc)
+		return -ENXIO;
+	clp = container_of(nc, struct nfs4_client, cl_nfsdfs);
+
+	ret = seq_open(file, &states_seq_ops);
+	if (ret)
+		return ret;
+	s = file->private_data;
+	s->private = clp;
+	return 0;
+}
+
+static int client_opens_release(struct inode *inode, struct file *file)
+{
+	struct seq_file *m = file->private_data;
+	struct nfs4_client *clp = m->private;
+
+	/* XXX: alternatively, we could get/drop in seq start/stop */
+	drop_client(clp);
+	return 0;
+}
+
+static const struct file_operations client_states_fops = {
+	.open		= client_states_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= client_opens_release,
+};
+
 static const struct tree_descr client_files[] = {
 	[0] = {"info", &client_info_fops, S_IRUSR},
-	[1] = {""},
+	[1] = {"states", &client_states_fops, S_IRUSR},
+	[3] = {""},
 };
 
 static struct nfs4_client *create_client(struct xdr_netobj name,
-- 
2.21.0


^ permalink raw reply related

* [PATCH 15/16] nfsd: create xdr_netobj_dup helper
From: J. Bruce Fields @ 2019-06-20 14:51 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields
In-Reply-To: <1561042275-12723-1-git-send-email-bfields@redhat.com>

From: "J. Bruce Fields" <bfields@redhat.com>

Move some repeated code to a common helper.  No change in behavior.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c        | 11 ++++-------
 include/linux/sunrpc/xdr.h |  7 +++++++
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 12e370e62453..8f35e440ef14 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1857,7 +1857,7 @@ static struct nfs4_client *alloc_client(struct xdr_netobj name)
 	clp = kmem_cache_zalloc(client_slab, GFP_KERNEL);
 	if (clp == NULL)
 		return NULL;
-	clp->cl_name.data = kmemdup(name.data, name.len, GFP_KERNEL);
+	xdr_netobj_dup(&clp->cl_name, &name, GFP_KERNEL);
 	if (clp->cl_name.data == NULL)
 		goto err_no_name;
 	clp->cl_ownerstr_hashtbl = kmalloc_array(OWNER_HASH_SIZE,
@@ -1867,7 +1867,6 @@ static struct nfs4_client *alloc_client(struct xdr_netobj name)
 		goto err_no_hashtbl;
 	for (i = 0; i < OWNER_HASH_SIZE; i++)
 		INIT_LIST_HEAD(&clp->cl_ownerstr_hashtbl[i]);
-	clp->cl_name.len = name.len;
 	INIT_LIST_HEAD(&clp->cl_sessions);
 	idr_init(&clp->cl_stateids);
 	atomic_set(&clp->cl_rpc_users, 0);
@@ -4000,12 +3999,11 @@ static inline void *alloc_stateowner(struct kmem_cache *slab, struct xdr_netobj
 	if (!sop)
 		return NULL;
 
-	sop->so_owner.data = kmemdup(owner->data, owner->len, GFP_KERNEL);
+	xdr_netobj_dup(&sop->so_owner, owner, GFP_KERNEL);
 	if (!sop->so_owner.data) {
 		kmem_cache_free(slab, sop);
 		return NULL;
 	}
-	sop->so_owner.len = owner->len;
 
 	INIT_LIST_HEAD(&sop->so_stateids);
 	sop->so_client = clp;
@@ -6093,12 +6091,11 @@ nfs4_set_lock_denied(struct file_lock *fl, struct nfsd4_lock_denied *deny)
 
 	if (fl->fl_lmops == &nfsd_posix_mng_ops) {
 		lo = (struct nfs4_lockowner *) fl->fl_owner;
-		deny->ld_owner.data = kmemdup(lo->lo_owner.so_owner.data,
-					lo->lo_owner.so_owner.len, GFP_KERNEL);
+		xdr_netobj_dup(&deny->ld_owner, &lo->lo_owner.so_owner,
+						GFP_KERNEL);
 		if (!deny->ld_owner.data)
 			/* We just don't care that much */
 			goto nevermind;
-		deny->ld_owner.len = lo->lo_owner.so_owner.len;
 		deny->ld_clientid = lo->lo_owner.so_client->cl_clientid;
 	} else {
 nevermind:
diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
index 9ee3970ba59c..8a87d8bcb197 100644
--- a/include/linux/sunrpc/xdr.h
+++ b/include/linux/sunrpc/xdr.h
@@ -164,6 +164,13 @@ xdr_decode_opaque_fixed(__be32 *p, void *ptr, unsigned int len)
 	return p + XDR_QUADLEN(len);
 }
 
+static inline void xdr_netobj_dup(struct xdr_netobj *dst,
+				  struct xdr_netobj *src, gfp_t gfp_mask)
+{
+	dst->data = kmemdup(src->data, src->len, gfp_mask);
+	dst->len = src->len;
+}
+
 /*
  * Adjust kvec to reflect end of xdr'ed data (RPC client XDR)
  */
-- 
2.21.0


^ permalink raw reply related

* [PATCH 06/16] nfsd4: add a client info file
From: J. Bruce Fields @ 2019-06-20 14:51 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields
In-Reply-To: <1561042275-12723-1-git-send-email-bfields@redhat.com>

From: "J. Bruce Fields" <bfields@redhat.com>

Add a new nfsd/clients/#/info file with some basic information about
each NFSv4 client.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c |  38 ++++++++++++++-
 fs/nfsd/nfsctl.c    | 114 +++++++++++++++++++++++++++++++++++++++++---
 fs/nfsd/nfsd.h      |   4 +-
 3 files changed, 148 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 370ee6f6b246..d3de89dacf89 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2214,6 +2214,41 @@ find_stateid_by_type(struct nfs4_client *cl, stateid_t *t, char typemask)
 	return s;
 }
 
+static int client_info_show(struct seq_file *m, void *v)
+{
+	struct inode *inode = m->private;
+	struct nfsdfs_client *nc;
+	struct nfs4_client *clp;
+	u64 clid;
+
+	nc = get_nfsdfs_client(inode);
+	if (!nc)
+		return -ENXIO;
+	clp = container_of(nc, struct nfs4_client, cl_nfsdfs);
+	memcpy(&clid, &clp->cl_clientid, sizeof(clid));
+	seq_printf(m, "clientid: 0x%llx\n", clid);
+	drop_client(clp);
+
+	return 0;
+}
+
+static int client_info_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, client_info_show, inode);
+}
+
+static const struct file_operations client_info_fops = {
+	.open		= client_info_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
+static const struct tree_descr client_files[] = {
+	[0] = {"info", &client_info_fops, S_IRUSR},
+	[1] = {""},
+};
+
 static struct nfs4_client *create_client(struct xdr_netobj name,
 		struct svc_rqst *rqstp, nfs4_verifier *verf)
 {
@@ -2242,7 +2277,8 @@ static struct nfs4_client *create_client(struct xdr_netobj name,
 	clp->cl_cb_session = NULL;
 	clp->net = net;
 	clp->cl_nfsd_dentry = nfsd_client_mkdir(nn, &clp->cl_nfsdfs,
-			clp->cl_clientid.cl_id - nn->clientid_base);
+			clp->cl_clientid.cl_id - nn->clientid_base,
+			client_files);
 	if (!clp->cl_nfsd_dentry) {
 		free_client(clp);
 		return NULL;
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 50c103c1aa13..185e9ee58481 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1207,14 +1207,116 @@ static struct dentry *nfsd_mkdir(struct dentry *parent, struct nfsdfs_client *nc
 	goto out;
 }
 
+static void clear_ncl(struct inode *inode)
+{
+	struct nfsdfs_client *ncl = inode->i_private;
+
+	inode->i_private = NULL;
+	synchronize_rcu();
+	kref_put(&ncl->cl_ref, ncl->cl_release);
+}
+
+
+struct nfsdfs_client *__get_nfsdfs_client(struct inode *inode)
+{
+	struct nfsdfs_client *nc = inode->i_private;
+
+	if (nc)
+		kref_get(&nc->cl_ref);
+	return nc;
+}
+
+struct nfsdfs_client *get_nfsdfs_client(struct inode *inode)
+{
+	struct nfsdfs_client *nc;
+
+	rcu_read_lock();
+	nc = __get_nfsdfs_client(inode);
+	rcu_read_unlock();
+	return nc;
+}
+/* from __rpc_unlink */
+static void nfsdfs_remove_file(struct inode *dir, struct dentry *dentry)
+{
+	int ret;
+
+	clear_ncl(d_inode(dentry));
+	dget(dentry);
+	ret = simple_unlink(dir, dentry);
+	d_delete(dentry);
+	dput(dentry);
+	WARN_ON_ONCE(ret);
+}
+
+static void nfsdfs_remove_files(struct dentry *root)
+{
+	struct dentry *dentry, *tmp;
+
+	list_for_each_entry_safe(dentry, tmp, &root->d_subdirs, d_child) {
+		if (!simple_positive(dentry)) {
+			WARN_ON_ONCE(1); /* I think this can't happen? */
+			continue;
+		}
+		nfsdfs_remove_file(d_inode(root), dentry);
+	}
+}
+
+/* XXX: cut'n'paste from simple_fill_super; figure out if we could share
+ * code instead. */
+static  int nfsdfs_create_files(struct dentry *root,
+					const struct tree_descr *files)
+{
+	struct inode *dir = d_inode(root);
+	struct inode *inode;
+	struct dentry *dentry;
+	int i;
+
+	inode_lock(dir);
+	for (i = 0; files->name && files->name[0]; i++, files++) {
+		if (!files->name)
+			continue;
+		dentry = d_alloc_name(root, files->name);
+		if (!dentry)
+			goto out;
+		inode = nfsd_get_inode(d_inode(root)->i_sb,
+					S_IFREG | files->mode);
+		if (!inode) {
+			dput(dentry);
+			goto out;
+		}
+		inode->i_fop = files->ops;
+		inode->i_private = __get_nfsdfs_client(dir);
+		d_add(dentry, inode);
+		fsnotify_create(dir, dentry);
+	}
+	inode_unlock(dir);
+	return 0;
+out:
+	nfsdfs_remove_files(root);
+	inode_unlock(dir);
+	return -ENOMEM;
+}
+
 /* on success, returns positive number unique to that client. */
-struct dentry *nfsd_client_mkdir(struct nfsd_net *nn, struct nfsdfs_client *ncl, u32 id)
+struct dentry *nfsd_client_mkdir(struct nfsd_net *nn,
+		struct nfsdfs_client *ncl, u32 id,
+		const struct tree_descr *files)
 {
+	struct dentry *dentry;
 	char name[11];
+	int ret;
 
 	sprintf(name, "%u", id);
 
-	return nfsd_mkdir(nn->nfsd_client_dir, ncl, name);
+	dentry = nfsd_mkdir(nn->nfsd_client_dir, ncl, name);
+	if (IS_ERR(dentry)) /* XXX: tossing errors? */
+		return NULL;
+	ret = nfsdfs_create_files(dentry, files);
+	if (ret) {
+		nfsd_client_rmdir(dentry);
+		return NULL;
+	}
+	return dentry;
 }
 
 /* Taken from __rpc_rmdir: */
@@ -1222,16 +1324,16 @@ void nfsd_client_rmdir(struct dentry *dentry)
 {
 	struct inode *dir = d_inode(dentry->d_parent);
 	struct inode *inode = d_inode(dentry);
-	struct nfsdfs_client *ncl = inode->i_private;
 	int ret;
 
-	inode->i_private = NULL;
-	synchronize_rcu();
-	kref_put(&ncl->cl_ref, ncl->cl_release);
+	inode_lock(dir);
+	nfsdfs_remove_files(dentry);
+	clear_ncl(inode);
 	dget(dentry);
 	ret = simple_rmdir(dir, dentry);
 	WARN_ON_ONCE(ret);
 	d_delete(dentry);
+	inode_unlock(dir);
 }
 
 static int nfsd_fill_super(struct super_block * sb, void * data, int silent)
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 85525dcbf77d..af2947551e9c 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -92,7 +92,9 @@ struct nfsdfs_client {
 	void (*cl_release)(struct kref *kref);
 };
 
-struct dentry *nfsd_client_mkdir(struct nfsd_net *nn, struct nfsdfs_client *ncl, u32 id);
+struct nfsdfs_client *get_nfsdfs_client(struct inode *);
+struct dentry *nfsd_client_mkdir(struct nfsd_net *nn,
+		struct nfsdfs_client *ncl, u32 id, const struct tree_descr *);
 void nfsd_client_rmdir(struct dentry *dentry);
 
 #if defined(CONFIG_NFSD_V2_ACL) || defined(CONFIG_NFSD_V3_ACL)
-- 
2.21.0


^ permalink raw reply related

* [PATCH 09/16] nfsd: add more information to client info file
From: J. Bruce Fields @ 2019-06-20 14:51 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields
In-Reply-To: <1561042275-12723-1-git-send-email-bfields@redhat.com>

From: "J. Bruce Fields" <bfields@redhat.com>

Add ip address, full client-provided identifier, and minor version.
There's much more that could possibly be useful but this is a start.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index dd89dc05f6ee..430cea90a715 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -42,6 +42,7 @@
 #include <linux/sunrpc/svcauth_gss.h>
 #include <linux/sunrpc/addr.h>
 #include <linux/jhash.h>
+#include <linux/string_helpers.h>
 #include "xdr4.h"
 #include "xdr4cb.h"
 #include "vfs.h"
@@ -2214,6 +2215,13 @@ find_stateid_by_type(struct nfs4_client *cl, stateid_t *t, char typemask)
 	return s;
 }
 
+static void seq_quote_mem(struct seq_file *m, char *data, int len)
+{
+	seq_printf(m, "\"");
+	seq_escape_mem_ascii(m, data, len);
+	seq_printf(m, "\"");
+}
+
 static int client_info_show(struct seq_file *m, void *v)
 {
 	struct inode *inode = m->private;
@@ -2227,6 +2235,10 @@ static int client_info_show(struct seq_file *m, void *v)
 	clp = container_of(nc, struct nfs4_client, cl_nfsdfs);
 	memcpy(&clid, &clp->cl_clientid, sizeof(clid));
 	seq_printf(m, "clientid: 0x%llx\n", clid);
+	seq_printf(m, "address: \"%pISpc\"\n", (struct sockaddr *)&clp->cl_addr);
+	seq_printf(m, "name: ");
+	seq_quote_mem(m, clp->cl_name.data, clp->cl_name.len);
+	seq_printf(m, "\nminor version: %d\n", clp->cl_minorversion);
 	drop_client(clp);
 
 	return 0;
-- 
2.21.0


^ permalink raw reply related

* [PATCH 03/16] nfsd4: use reference count to free client
From: J. Bruce Fields @ 2019-06-20 14:51 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields
In-Reply-To: <1561042275-12723-1-git-send-email-bfields@redhat.com>

From: "J. Bruce Fields" <bfields@redhat.com>

Keep a second reference count which is what is really used to decide
when to free the client's memory.

Next I'm going to add an nfsd/clients/ directory with a subdirectory for
each NFSv4 client.  File objects under nfsd/clients/ will hold these
references.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c | 26 +++++++++++++++++++++-----
 fs/nfsd/state.h     |  1 +
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 2a13b6cbb695..d2ea41add566 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1879,6 +1879,24 @@ static struct nfs4_client *alloc_client(struct xdr_netobj name)
 	return NULL;
 }
 
+static void __free_client(struct kref *k)
+{
+	struct nfs4_client *clp = container_of(k, struct nfs4_client, cl_ref);
+
+	free_svc_cred(&clp->cl_cred);
+	kfree(clp->cl_ownerstr_hashtbl);
+	kfree(clp->cl_name.data);
+	idr_destroy(&clp->cl_stateids);
+	if (clp->cl_nfsd_dentry)
+		nfsd_client_rmdir(clp->cl_nfsd_dentry);
+	kmem_cache_free(client_slab, clp);
+}
+
+void drop_client(struct nfs4_client *clp)
+{
+	kref_put(&clp->cl_ref, __free_client);
+}
+
 static void
 free_client(struct nfs4_client *clp)
 {
@@ -1891,11 +1909,7 @@ free_client(struct nfs4_client *clp)
 		free_session(ses);
 	}
 	rpc_destroy_wait_queue(&clp->cl_cb_waitq);
-	free_svc_cred(&clp->cl_cred);
-	kfree(clp->cl_ownerstr_hashtbl);
-	kfree(clp->cl_name.data);
-	idr_destroy(&clp->cl_stateids);
-	kmem_cache_free(client_slab, clp);
+	drop_client(clp);
 }
 
 /* must be called under the client_lock */
@@ -2216,6 +2230,8 @@ static struct nfs4_client *create_client(struct xdr_netobj name,
 		free_client(clp);
 		return NULL;
 	}
+
+	kref_init(&clp->cl_ref);
 	nfsd4_init_cb(&clp->cl_cb_null, clp, NULL, NFSPROC4_CLNT_CB_NULL);
 	clp->cl_time = get_seconds();
 	clear_bit(0, &clp->cl_cb_slot_busy);
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index f79ad7202e82..8eacdbc50cd7 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -348,6 +348,7 @@ struct nfs4_client {
 	u32			cl_exchange_flags;
 	/* number of rpc's in progress over an associated session: */
 	atomic_t		cl_rpc_users;
+	struct kref		cl_ref;
 	struct nfs4_op_map      cl_spo_must_allow;
 
 	/* for nfs41 callbacks */
-- 
2.21.0


^ permalink raw reply related

* [PATCH 01/16] nfsd: persist nfsd filesystem across mounts
From: J. Bruce Fields @ 2019-06-20 14:51 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields
In-Reply-To: <1561042275-12723-1-git-send-email-bfields@redhat.com>

From: "J. Bruce Fields" <bfields@redhat.com>

Keep around one internal mount of the nfsd filesystem so that we can add
stuff to it when clients come and go, regardless of whether anyone has
it mounted.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/netns.h  |  3 +++
 fs/nfsd/nfsctl.c | 13 +++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
index 789abc4dd1d2..e7890e87d30b 100644
--- a/fs/nfsd/netns.h
+++ b/fs/nfsd/netns.h
@@ -55,6 +55,9 @@ struct nfsd_net {
 	bool grace_ended;
 	time_t boot_time;
 
+	/* internal mount of the "nfsd" pseudofilesystem: */
+	struct vfsmount *nfsd_mnt;
+
 	/*
 	 * reclaim_str_hashtbl[] holds known client info from previous reset/reboot
 	 * used in reboot/reset lease grace period processing
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 90972e1fd785..ab1fb81b7f5e 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1231,6 +1231,7 @@ unsigned int nfsd_net_id;
 static __net_init int nfsd_init_net(struct net *net)
 {
 	int retval;
+	struct vfsmount *mnt;
 	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
 
 	retval = nfsd_export_init(net);
@@ -1251,8 +1252,17 @@ static __net_init int nfsd_init_net(struct net *net)
 
 	atomic_set(&nn->ntf_refcnt, 0);
 	init_waitqueue_head(&nn->ntf_wq);
+
+	mnt =  vfs_kern_mount(&nfsd_fs_type, SB_KERNMOUNT, "nfsd", NULL);
+	if (IS_ERR(mnt)) {
+		retval = PTR_ERR(mnt);
+		goto out_mount_err;
+	}
+	nn->nfsd_mnt = mnt;
 	return 0;
 
+out_mount_err:
+	nfsd_idmap_shutdown(net);
 out_idmap_error:
 	nfsd_export_shutdown(net);
 out_export_error:
@@ -1261,6 +1271,9 @@ static __net_init int nfsd_init_net(struct net *net)
 
 static __net_exit void nfsd_exit_net(struct net *net)
 {
+	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
+
+	mntput(nn->nfsd_mnt);
 	nfsd_idmap_shutdown(net);
 	nfsd_export_shutdown(net);
 	nfsd_netns_free_versions(net_generic(net, nfsd_net_id));
-- 
2.21.0


^ permalink raw reply related

* [PATCH RFC v1 7/7] serial: imx: get rid of imx_uart_rts_auto()
From: Sergey Organov @ 2019-06-20 14:47 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Pengutronix Kernel Team, NXP Linux Team, linux-arm-kernel,
	linux-serial, Uwe Kleine-König
In-Reply-To: <1561042073-617-1-git-send-email-sorganov@gmail.com>

Called in only one place, for RS232, it only obscures things, as it
doesn't go well with 2 similar named functions,
imx_uart_rts_inactive() and imx_uart_rts_active(), that both are
RS485-specific.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 drivers/tty/serial/imx.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 171347d..a5e80a0 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -402,13 +402,6 @@ static void imx_uart_rts_inactive(struct imx_port *sport, u32 *ucr2)
 	mctrl_gpio_set(sport->gpios, sport->port.mctrl);
 }
 
-/* called with port.lock taken and irqs caller dependent */
-static void imx_uart_rts_auto(struct imx_port *sport, u32 *ucr2)
-{
-	if (*ucr2 & UCR2_CTS)
-		*ucr2 |= UCR2_CTSC;
-}
-
 /* called with port.lock taken and irqs off */
 static void imx_uart_start_rx(struct uart_port *port)
 {
@@ -1598,8 +1591,10 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 		else
 			imx_uart_rts_inactive(sport, &ucr2);
 
-	} else if (termios->c_cflag & CRTSCTS)
-		imx_uart_rts_auto(sport, &ucr2);
+	} else if (termios->c_cflag & CRTSCTS) {
+		if (ucr2 & UCR2_CTS)
+			ucr2 |= UCR2_CTSC;
+	}
 
 	if (termios->c_cflag & CRTSCTS)
 		ucr2 &= ~UCR2_IRTS;
-- 
2.10.0.1.g57b01a3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH RFC v1 6/7] serial: imx: set_mctrl(): correctly restore autoRTS state
From: Sergey Organov @ 2019-06-20 14:47 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Pengutronix Kernel Team, NXP Linux Team, linux-arm-kernel,
	linux-serial, Uwe Kleine-König
In-Reply-To: <1561042073-617-1-git-send-email-sorganov@gmail.com>

imx_uart_set_mctrl() happened to set UCR2_CTSC bit whenever TIOCM_RTS
was set, no matter if RTS/CTS handshake is enabled or not. Now fixed by
turning handshake on only when CRTSCTS bit for the port is set.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 drivers/tty/serial/imx.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 4867f80..171347d 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -970,10 +970,19 @@ static void imx_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
 	if (!(port->rs485.flags & SER_RS485_ENABLED)) {
 		u32 ucr2;
 
+		/*
+		 * Turn off autoRTS (UCR2_CTSC) if RTS is lowered and restore
+		 * autoRTS setting if RTS is raised. Inverted UCR2_IRTS is set
+		 * if and only if CRTSCTS bit is set for the port, so we use it
+		 * to get the state to restore to.
+		 */
 		ucr2 = imx_uart_readl(sport, UCR2);
 		ucr2 &= ~(UCR2_CTS | UCR2_CTSC);
-		if (mctrl & TIOCM_RTS)
-			ucr2 |= UCR2_CTS | UCR2_CTSC;
+		if (mctrl & TIOCM_RTS) {
+			ucr2 |= UCR2_CTS;
+			if (!(ucr2 & UCR2_IRTS))
+				ucr2 |= UCR2_CTSC;
+		}
 		imx_uart_writel(sport, ucr2, UCR2);
 	}
 
-- 
2.10.0.1.g57b01a3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH -next] ASoC: SOF: Intel: hda: remove duplicated include from hda.c
From: YueHaibing @ 2019-06-20 14:57 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	Bard Liao, Arnd Bergmann, Pierre-Louis Bossart, Zhu Yingjiang,
	Kai Vehmanen
  Cc: YueHaibing, alsa-devel, linux-kernel, kernel-janitors

Remove duplicated include.

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 sound/soc/sof/intel/hda.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index 51c1c1787de7..7f665392618f 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -19,7 +19,6 @@
 #include <sound/hda_register.h>
 
 #include <linux/module.h>
-#include <sound/hdaudio_ext.h>
 #include <sound/sof.h>
 #include <sound/sof/xtensa.h>
 #include "../ops.h"






^ permalink raw reply related

* [PATCH RFC v1 5/7] serial: imx: set_termios(): do not enable autoRTS if RTS is unset
From: Sergey Organov @ 2019-06-20 14:47 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Pengutronix Kernel Team, NXP Linux Team, linux-arm-kernel,
	linux-serial, Uwe Kleine-König
In-Reply-To: <1561042073-617-1-git-send-email-sorganov@gmail.com>

set_termios() shouldn't set UCR2_CTSC bit if UCR2_CTS (=TIOCM_RTS) is
cleared. Added corresponding check in imx_uart_rts_auto() to fix this.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 drivers/tty/serial/imx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index e0f5365..4867f80 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -405,7 +405,8 @@ static void imx_uart_rts_inactive(struct imx_port *sport, u32 *ucr2)
 /* called with port.lock taken and irqs caller dependent */
 static void imx_uart_rts_auto(struct imx_port *sport, u32 *ucr2)
 {
-	*ucr2 |= UCR2_CTSC;
+	if (*ucr2 & UCR2_CTS)
+		*ucr2 |= UCR2_CTSC;
 }
 
 /* called with port.lock taken and irqs off */
-- 
2.10.0.1.g57b01a3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v2 2/2] ssh: Add interface ssh_search_dir
From: Dominick Grift @ 2019-06-20 14:50 UTC (permalink / raw)
  To: Alexander Miroshnichenko; +Cc: selinux-refpolicy
In-Reply-To: <20190620144138.15172-3-alex@millerson.name>

[-- Attachment #1: Type: text/plain, Size: 1587 bytes --]

On Thu, Jun 20, 2019 at 05:41:38PM +0300, Alexander Miroshnichenko wrote:
> Create interface ssh_search_dir to allow ssh_server search for keys in non-standard location.
> 
> Signed-off-by: Alexander Miroshnichenko <alex@millerson.name>
> ---
>  policy/modules/services/ssh.if | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/policy/modules/services/ssh.if b/policy/modules/services/ssh.if
> index 0941f133711e..51c64ded00c4 100644
> --- a/policy/modules/services/ssh.if
> +++ b/policy/modules/services/ssh.if
> @@ -680,6 +680,24 @@ interface(`ssh_agent_exec',`
>  	can_exec($1, ssh_agent_exec_t)
>  ')
>  
> +########################################
> +## <summary>
> +##      Search for keys in non-standard location
> +## </summary>
> +## <param name="domain">
> +##      <summary>
> +##      Domain allowed access.
> +##      </summary>
> +## </param>
> +#
> +interface(`ssh_search_dir',`
> +        gen_require(`
> +                type sshd_t;
> +        ')
> +
> +	allow sshd_t $1:dir search_dir_perms;

This is generally not allowed. The caller should generally be the source.
Regardless of the above. Keys should be in user home directories. I wonder what specific scenario prompted you to propose this interface?

> +')
> +
>  ########################################
>  ## <summary>
>  ##	Read ssh home directory content
> -- 
> 2.21.0
> 

-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply

* Re: drm connectors, tegra, and the web they weave (was Re: [PATCH 58/59] drm/todo: Add new debugfs todo)
From: Thierry Reding @ 2019-06-20 14:50 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Greg Kroah-Hartman, Intel Graphics Development, DRI Development,
	Jonathan Hunter, Daniel Vetter, linux-tegra, Daniel Vetter
In-Reply-To: <20190618214656.GH12905@phenom.ffwll.local>


[-- Attachment #1.1: Type: text/plain, Size: 3000 bytes --]

On Tue, Jun 18, 2019 at 11:46:56PM +0200, Daniel Vetter wrote:
> On Tue, Jun 18, 2019 at 08:01:13PM +0200, Greg Kroah-Hartman wrote:
> > On Tue, Jun 18, 2019 at 07:32:20PM +0200, Daniel Vetter wrote:
> > > On Tue, Jun 18, 2019 at 5:25 PM Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > > > On Tue, Jun 18, 2019 at 05:19:38PM +0200, Greg Kroah-Hartman wrote:
> > > > > I could just "open code" a bunch of calls to debugfs_create_file() for
> > > > > these drivers, which would solve this issue, but in a more "non-drm"
> > > > > way.  Is it worth to just do that instead of overthinking the whole
> > > > > thing and trying to squish it into the drm "model" of drm debugfs calls?
> > > >
> > > > An example of "open coding" this is the patch below for the sor.c
> > > > driver.
> > > 
> > > I think open-coding is the way to go here. One of the todos is to
> > > extend debugfs support for crtc/connectors, but looking at the
> > > open-coded version we really don't need a drm-flavoured midlayer here.
> > 
> > There already is debugfs support in the code for crtc/connectors, these
> > files are "hanging" off of those locations already.  I'll keep that, but
> > indent it one more directory so that there's no namespace collisions.
> 
> The todo was to have some drm wrappers here for the boilerplate, but after
> looking at your version that's not a good idea. So not just making sure
> crtcs/connectors have a debugfs directory made for them, but more.
> 
> Wrt adding a new directory: debugfs isnt uapi, but there's usually a
> massive pile of script relying on them, so it's not nice to shuffle paths
> around. Plus the lifetimes match anyway (at least if you don't hotplug
> connectors, which tegra doesn't do).

So, I think you two already covered everything. From a Tegra perspective
there's not really a need to care about the exact structure of debugfs
because there are only a handful of scripts that use this and they are
not exactly widely distributed. At the same time there's really no need
to add another level of directories, since the connector really is the
SOR, so an sor directory in the connector's directory is just redundant.
Cleaning up and lifetime management aren't issues, really, so there are
no good arguments for adding it, in my opinion.

Historically, the sor.c driver is interesting because it used to be just
plain debugfs calls. Only when I added a second debugfs file I decided
to go with the built-in DRM infrastructure for this and then later went
and converted the first file to it as well, for consistency. I do
remember though that I was very unhappy about the fact that I had to do
all this kmemdup()'ing just to make debugfs files per-instance (the DRM
infrastructure doesn't allow that by default). In retrospect that was a
poor decision and I should've just stuck with debugfs and perhaps just
spin a couple of helpers around that instead.

So I'm happy to see this effort.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [Qemu-devel] [PATCH v3 2/4] hw/firmware: Add Edk2Crypto and edk2_add_host_crypto_policy()
From: Laszlo Ersek @ 2019-06-20 13:51 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Gerd Hoffmann, Michael S. Tsirkin,
	qemu-devel
  Cc: Daniel P . Berrange, Markus Armbruster
In-Reply-To: <88b7370c-1b6f-11e0-9fbe-d532e5726aa6@redhat.com>

On 06/20/19 14:07, Philippe Mathieu-Daudé wrote:
> Hi Laszlo,
> 
> On 3/13/19 11:11 AM, Laszlo Ersek wrote:
>> On 03/13/19 10:43, Laszlo Ersek wrote:
>>> On 03/10/19 01:47, Philippe Mathieu-Daudé wrote:
>>>> The Edk2Crypto object is used to hold configuration values specific
>>>> to EDK2.
>>>>
>>>> The edk2_add_host_crypto_policy() function loads crypto policies
>>>> from the host, and register them as fw_cfg named file items.
>>>> So far only the 'https' policy is supported.
>>>>
>>>> A usercase example is the 'HTTPS Boof' feature of OVMF [*].
>>>>
>>>> Usage example:
>>>>
>>>>   $ qemu-system-x86_64 \
>>>>       --object edk2_crypto,id=https,\
>>>>               ciphers=/etc/crypto-policies/back-ends/openssl.config,\
>>>>               cacerts=/etc/pki/ca-trust/extracted/edk2/cacerts.bin
>>>>
>>>> (On Fedora these files are provided by the ca-certificates and
>>>> crypto-policies packages).
>>>>
>>>> [*]: https://github.com/tianocore/edk2/blob/master/OvmfPkg/README
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> ---
>>>> v3:
>>>> - '-object' -> '--object' in commit description (Eric)
>>>> - reworded the 'TODO: g_free' comment
>>>> ---
>>>>  MAINTAINERS                             |   8 ++
>>>>  hw/Makefile.objs                        |   1 +
>>>>  hw/firmware/Makefile.objs               |   1 +
>>>>  hw/firmware/uefi_edk2_crypto_policies.c | 177 ++++++++++++++++++++++++
>>>>  include/hw/firmware/uefi_edk2.h         |  28 ++++
>>>>  5 files changed, 215 insertions(+)
>>>>  create mode 100644 hw/firmware/Makefile.objs
>>>>  create mode 100644 hw/firmware/uefi_edk2_crypto_policies.c
>>>>  create mode 100644 include/hw/firmware/uefi_edk2.h
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index cf09a4c127..70122b3d0d 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -2206,6 +2206,14 @@ F: include/hw/i2c/smbus_master.h
>>>>  F: include/hw/i2c/smbus_slave.h
>>>>  F: include/hw/i2c/smbus_eeprom.h
>>>>  
>>>> +EDK2 Firmware
>>>> +M: Laszlo Ersek <lersek@redhat.com>
>>>> +M: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> +S: Maintained
>>>> +F: docs/interop/firmware.json
>>>> +F: hw/firmware/uefi_edk2_crypto_policies.c
>>>> +F: include/hw/firmware/uefi_edk2.h
>>>> +
>>>
>>> I'm not happy with this.
>>>
>>> First, "docs/interop/firmware.json" is meant for more than just EDK2. We
>>> shouldn't list it in a section called "EDK2 Firmware". I can't suggest
>>> an alternative (MAINTAINERS is *huge* -- 2500+ lines), but this one
>>> would be misleading.
>>>
>>> Second, we expose fw_cfg files to edk2 platform firmware from a bunch of
>>> other places. For example -- and in this case I do mean to provide a
>>> complex example! --, see "etc/smi/supported-features",
>>> "etc/smi/requested-features", and "etc/smi/features-ok", in file
>>> "hw/isa/lpc_ich9.c". I'm unconvinced that the present feature merits new
>>> directories and new files.
>>>
>>> Then again, I also don't know where to put the logic. I guess I'll have
>>> to defer to more experienced reviewers.
>>>
>>> [snipping lots of QOM boilerplate]
>>>
>>>> +void edk2_add_host_crypto_policy(FWCfgState *fw_cfg)
>>>> +{
>>>> +    Edk2Crypto *s;
>>>> +
>>>> +    s = edk2_crypto_by_id("https", NULL);
>>>> +    if (!s) {
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    if (s->ciphers_path) {
>>>> +        /*
>>>> +         * Note:
>>>> +         * Unlike with fw_cfg_add_file() where the allocated data has
>>>> +         * to be valid for the lifetime of the FwCfg object, there is
>>>> +         * no such contract interface with fw_cfg_add_file_from_host().
>>>> +         * It would be easier that the FwCfg object keeps reference of
>>>> +         * its allocated memory and releases it when destroy, but it
>>>> +         * currently doesn't. Meanwhile we simply add this TODO comment.
>>>> +         */
>>>> +        fw_cfg_add_file_from_host(fw_cfg, "etc/edk2/https/ciphers",
>>>> +                                  s->ciphers_path, NULL);
>>>> +    }
>>>> +
>>>> +    if (s->cacerts_path) {
>>>> +        /*
>>>> +         * TODO: g_free the returned pointer
>>>> +         * (see previous comment for ciphers_path in this function).
>>>> +         */
>>>> +        fw_cfg_add_file_from_host(fw_cfg, "etc/edk2/https/cacerts",
>>>> +                                  s->cacerts_path, NULL);
>>>> +    }
>>>> +}
>>>
>>> Shouldn't we do some error checking here?
>>>
>>> I mean, printing an error message in fw_cfg_add_file_from_host(), and
>>> then continuing without exposing the named files in question to the
>>> firmware, could be OK if this was a "default on" feature. But (IIUC)
>>> here the user provided an explicit "-object" option, and we've just
>>> failed to construct the object. Doesn't such a situation usually prevent
>>> QEMU startup?
>>
>> Wait, I could be totally confused here. (Returning to this patch after
>> seeing the rest of the series.)
>>
>> Is it actually the case that the Edk2Crypto object holds nothing more
>> than two pathnames -- and so its construction can virtually never fail?
>> While the actual fw_cfg population occurs separately, in a machine_done
>> notifier?
>>
>> If that's the case, I don't think it's the right approach. Reading the
>> host files, and populating fw_cfg with them, should be part of the
>> object construction. And if those steps fail, the object should not be
>> possible to construct.
>>
>> We did something similar with the vmgenid device [hw/acpi/vmgenid.c], if
>> I remember correctly. It also has a dependency on fw_cfg...
>>
>> Ahh, no, I'm absolutely wrong about that. vmgenid_realize() doesn't do
>> anything with fw_cfg. Instead, we have acpi_setup() in
>> "hw/i386/acpi-build.c", which calls find_vmgenid_dev() and
>> vmgenid_add_fw_cfg(). And acpi_setup() is certainly called from
>> pc_machine_done().
>>
>> In other words, the pattern that you use here matches existing practice.
>> Realize the device (or object) first, then add the fw_cfg thingies in
>> the "machine done" callback. OK.
>>
>> *Still*, I would like to see better error handling/reporting (or an
>> explanation why I'm wrong). How about reworking the edk2crypto class
>> itself -- it shouldn't just hold the pathnames of the files, but also
>> their contents. That is:
>>
>> - g_file_get_contents() should be called in the realize method
>> - the object would own the file contents
>> - the realize method would ensure that there wouldn't be any other
>> instance of the class (i.e. that it would be a singleton -- see the same
>> idea in vmgenid)
>> - there would be no need for the fw_cfg_add_file_from_host() API
>> - the machine done notifier would be extended to locate the object
>> (there could be zero or one instances), and if the one instance were
>> found, the machine done callback would hook the file contents into
>> fw_cfg. fw_cfg_add_file() cannot fail, so no errors to report at this stage.
>>
>> Again I think this would follow the pattern from vmgenid.
> 
> I want to say I am impressed by your deep review. Your design is
> obviously way cleaner/safer. I think I was missing some part of the big
> picture here, thank you for your detailed comments!
> 
> I did not know how vmgenid is processed. The only difference is I don't
> want the edk2crypto class to be a device, but rather a simple user
> object, and we already have an interface that does that:
> TYPE_USER_CREATABLE. Its UserCreatableClass::complete() method is
> similar to DeviceClass::realize() in managing errors at object
> instantiation, so the machine done notifier never fails.
> I'll respin.

Sounds good to me, thanks! Please do CC reviewers with QOM experience,
as I'm not familiar with TYPE_USER_CREATABLE.

Thanks!
Laszlo


^ permalink raw reply

* [PATCH] SUNRPC: Fix a credential refcount leak
From: Trond Myklebust @ 2019-06-20 14:47 UTC (permalink / raw)
  To: Anna Schumaker; +Cc: Ido Schimmel, linux-nfs

All callers of __rpc_clone_client() pass in a value for args->cred,
meaning that the credential gets assigned and referenced in
the call to rpc_new_client().

Reported-by: Ido Schimmel <idosch@idosch.org>
Fixes: 79caa5fad47c ("SUNRPC: Cache cred of process creating the rpc_client")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
---
 net/sunrpc/clnt.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 627a87a71f8b..010e59cf2ae0 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -634,7 +634,6 @@ static struct rpc_clnt *__rpc_clone_client(struct rpc_create_args *args,
 	new->cl_discrtry = clnt->cl_discrtry;
 	new->cl_chatty = clnt->cl_chatty;
 	new->cl_principal = clnt->cl_principal;
-	new->cl_cred = get_cred(clnt->cl_cred);
 	return new;
 
 out_err:
-- 
2.21.0


^ permalink raw reply related

* [PATCH RFC v1 4/7] serial: imx: set_termios(): preserve RTS state
From: Sergey Organov @ 2019-06-20 14:47 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Pengutronix Kernel Team, NXP Linux Team, linux-arm-kernel,
	linux-serial, Uwe Kleine-König
In-Reply-To: <1561042073-617-1-git-send-email-sorganov@gmail.com>

imx_set_termios() cleared RTS on every call, now fixed.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 drivers/tty/serial/imx.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 17e2322..e0f5365 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1563,7 +1563,14 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	spin_lock_irqsave(&sport->port.lock, flags);
 
-	ucr2 = UCR2_SRST | UCR2_IRTS;
+	/*
+	 * Read current UCR2 and save it for future use, then clear all the bits
+	 * except those we will or may need to preserve.
+	 */
+	old_ucr2 = imx_uart_readl(sport, UCR2);
+	ucr2 = old_ucr2 & (UCR2_TXEN | UCR2_RXEN | UCR2_ATEN | UCR2_CTS);
+
+	ucr2 |= UCR2_SRST | UCR2_IRTS;
 	if ((termios->c_cflag & CSIZE) == CS8)
 		ucr2 |= UCR2_WS;
 
@@ -1632,7 +1639,6 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 	imx_uart_writel(sport,
 			old_ucr1 & ~(UCR1_TXMPTYEN | UCR1_RRDYEN | UCR1_RTSDEN),
 			UCR1);
-	old_ucr2 = imx_uart_readl(sport, UCR2);
 	imx_uart_writel(sport, old_ucr2 & ~UCR2_ATEN, UCR2);
 
 	while (!(imx_uart_readl(sport, USR2) & USR2_TXDC))
@@ -1640,7 +1646,6 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	/* then, disable everything */
 	imx_uart_writel(sport, old_ucr2 & ~(UCR2_TXEN | UCR2_RXEN | UCR2_ATEN), UCR2);
-	old_ucr2 &= (UCR2_TXEN | UCR2_RXEN | UCR2_ATEN);
 
 	/* custom-baudrate handling */
 	div = sport->port.uartclk / (baud * 16);
@@ -1678,8 +1683,7 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	imx_uart_writel(sport, old_ucr1, UCR1);
 
-	/* set the parity, stop bits and data size */
-	imx_uart_writel(sport, ucr2 | old_ucr2, UCR2);
+	imx_uart_writel(sport, ucr2, UCR2);
 
 	if (UART_ENABLE_MS(&sport->port, termios->c_cflag))
 		imx_uart_enable_ms(&sport->port);
-- 
2.10.0.1.g57b01a3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH RFC v1 3/7] serial: imx: set_termios(): clarify RTS/CTS bits calculation
From: Sergey Organov @ 2019-06-20 14:47 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Pengutronix Kernel Team, NXP Linux Team, linux-arm-kernel,
	linux-serial, Uwe Kleine-König
In-Reply-To: <1561042073-617-1-git-send-email-sorganov@gmail.com>

Avoid repeating the same code for rs485 twice.

Make it obvious we clear CRTSCTS bit in termios->c_cflag whenever
sport->have_rtscts is false.

Make it obvious we clear UCR2_IRTS whenever CRTSCTS is set.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 drivers/tty/serial/imx.c | 36 +++++++++++++-----------------------
 1 file changed, 13 insertions(+), 23 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 87802fd..17e2322 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1567,35 +1567,25 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 	if ((termios->c_cflag & CSIZE) == CS8)
 		ucr2 |= UCR2_WS;
 
-	if (termios->c_cflag & CRTSCTS) {
-		if (sport->have_rtscts) {
-			ucr2 &= ~UCR2_IRTS;
+	if (!sport->have_rtscts)
+		termios->c_cflag &= ~CRTSCTS;
 
-			if (port->rs485.flags & SER_RS485_ENABLED) {
-				/*
-				 * RTS is mandatory for rs485 operation, so keep
-				 * it under manual control and keep transmitter
-				 * disabled.
-				 */
-				if (port->rs485.flags &
-				    SER_RS485_RTS_AFTER_SEND)
-					imx_uart_rts_active(sport, &ucr2);
-				else
-					imx_uart_rts_inactive(sport, &ucr2);
-			} else {
-				imx_uart_rts_auto(sport, &ucr2);
-			}
-		} else {
-			termios->c_cflag &= ~CRTSCTS;
-		}
-	} else if (port->rs485.flags & SER_RS485_ENABLED) {
-		/* disable transmitter */
+	if (port->rs485.flags & SER_RS485_ENABLED) {
+		/*
+		 * RTS is mandatory for rs485 operation, so keep
+		 * it under manual control and keep transmitter
+		 * disabled.
+		 */
 		if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
 			imx_uart_rts_active(sport, &ucr2);
 		else
 			imx_uart_rts_inactive(sport, &ucr2);
-	}
 
+	} else if (termios->c_cflag & CRTSCTS)
+		imx_uart_rts_auto(sport, &ucr2);
+
+	if (termios->c_cflag & CRTSCTS)
+		ucr2 &= ~UCR2_IRTS;
 
 	if (termios->c_cflag & CSTOPB)
 		ucr2 |= UCR2_STPB;
-- 
2.10.0.1.g57b01a3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v2 bpf-next 00/11] BTF-defined BPF map definitions
From: Lorenz Bauer @ 2019-06-20 14:49 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Daniel Borkmann, Andrii Nakryiko, Alexei Starovoitov, Networking,
	bpf, Kernel Team, Jakub Kicinski, Joe Stringer
In-Reply-To: <CAEf4Bzae1CPDkhPrESa2ZmiOH8Mqf0KA_4ty9z=xnYn=q7Frhw@mail.gmail.com>

On Tue, 18 Jun 2019 at 22:37, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:

> > I would just drop the object-scope pinning. We avoided using it and I'm not
> > aware if anyone else make use. It also has the ugly side-effect that this
> > relies on AF_ALG which e.g. on some cloud provider shipped kernels is disabled.
> > The pinning attribute should be part of the standard set of map attributes for
> > libbpf though as it's generally useful for networking applications.
>
> Sounds good. I'll do some more surveying of use cases inside FB to see
> if anyone needs object-scope pinning, just to be sure we are not
> short-cutting anyone.

I'm also curious what the use cases for declarative pinning are. From my
limited POV it doesn't seem that useful? There are a couple of factors:

* Systemd mounts the default location only accessible to root, so I have to
  used my own bpffs mount.
* Since I don't want to hard code that, I put it in a config file.
* After loading the ELF we pin maps from the daemon managing the XDP.

How do other people work around this? Hard coding it in the ELF seems
suboptimal.

> > And the loader should figure this out and combine everything in the background.
> > Otherwise above 'struct inner_map_t value' would be mixing convention of using
> > pointer vs non-pointer which may be even more confusing.
>
> There are two reasons I didn't want to go with that approach:
>
> 1. This syntax makes my_inner_map usable as a stand-alone map, while
> it's purpose is to serve as a inner map prototype. While technically
> it is ok to use my_inner_map as real map, it's kind of confusing and
> feels unclean.

I agree, avoiding this problem is good.

> So we came up with a way to "encode" integer constants as part of BTF
> type information, so that *all* declarative information is part of BTF
> type, w/o the need to compile-time initialization. We tried to go the
> other way (what Jakub was pushing for), but we couldn't figure out
> anything that would work w/o more compiler hacks. So here's the
> updated proposal:
>
> #define __int(name, val) int (*name)[val]

Consider my mind blown: https://cdecl.org/?q=int+%28*foo%29%5B10%5D

> #define __type(name, val) val (*foo)

Maybe it's enough to just hide the pointer-ness?

  #define __member(name) (*name)
  struct my_value __member(value);

> struct my_inner_map {
>         __int(type, BPF_MAP_TYPE_ARRAY);
>         __int(max_entries, 1000);
>         __type(key, int);
>         __type(value, struct my_value);

What if this did

  __type(value, struct my_value)[1000];
  struct my_value __member(value)[1000]; // alternative

instead, and skipped max_entries?

> static struct {
>         __int(type, BPF_MAP_TYPE_ARRAY_OF_MAPS);
>         __int(max_entries, 1000);
>         __type(key, int);
>         __type(value, struct my_inner_map);
>         struct my_inner_map *values[];
> } my_initialized_outer_map SEC(".maps") = {
>         .values = {
>                 &imap1,
>                 [500] = &imap2,
>         },
> };
>
> Here struct my_inner_map is complete definition of array map w/ 1000
> elements w/ all the type info for k/v. That struct is used as a
> template for my_outer_map map-in-map. my_initialized_outer_map is the
> case of pre-initialization of array-of-maps w/ instances of existing
> maps imap1 and imap2.

For my_initialized_outer_map, which section does .values end up in the
generated ELF? How much space is going to be allocated? 501 * 4 bytes?

> The idea is that we encode integer fields as array dimensions + use
> pointer to an array to save space. Given that syntax in plain C is a
> bit ugly and hard to remember, we hide that behind __int macro. Then
> in line with __int, we also have __type macro, that hides that hateful
> pointer for key/value types. This allows map definition to be
> self-describing w/o having to look at initialized ELF data section at
> all, except for special cases of explicitly initializing map-in-map or
> prog_array.
>
> What do you think?

I think this is an interesting approach. One thing I'm not sure of is handling
these types from C. For example:

  sizeof(my_outer_map.value)

This compiles, but doesn't produce the intended result. Correct would be:

  sizeof(my_outer_map.value[0])

At that point you have to understand that value is a pointer so all of
our efforts
are for naught. I suspect there is other weirdness like this, but I need to play
with it a little bit more.

> Yeah I can definitely see some confusion here. But it seems like this
> is more of a semantics of map sharing, and maybe it should be some
> extra option for when we have automatic support for extern (shared)
> maps. E.g., something like
>
> __int(sharing, SHARE_STRATEGY_MERGE) vs __int(sharing, SHARE_STRATEGY_OVERWRITE)
>
> Haven't though through exact syntax, naming, semantics, but it seems
> doable to support both, depending on desired behavior.
>
> Maybe we should also unify this w/ pinning? E.g., there are many
> sensible ways to handle already existing pinned map:
>
> 1. Reject program (e.g., if BPF application is the source of truth for that map)
> 2. Use pinned as is (e.g., if BPF application wants to consume data
> from source of truth app)
> 3. Merge (what you described above)
> 4. Replace/reset - not sure if useful/desirable.

From my experience, trying to support many use cases in a purely declarative
fashion ends up creating many edge cases, and quirky behaviour that is hard to
fix later on. It's a bit like merging dictionaries in $LANGUAGE,
which starts out simple and then gets complicated because sometimes you
want to override a key, but lists should be concatenated, except in
that one case...

I wonder: are there many use cases where writing some glue code isn't
possible? With libbpf getting more mature APIs that should become easier and
easier. We could probably support existing iproute2 features that way as well.

-- 
Lorenz Bauer  |  Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK

www.cloudflare.com

^ permalink raw reply

* Re: [PATCH v2] mm, memcg: Add a memcg_slabinfo debugfs file
From: Waiman Long @ 2019-06-20 14:48 UTC (permalink / raw)
  To: Shakeel Butt
  Cc: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
	Andrew Morton, Linux MM, LKML, Michal Hocko, Roman Gushchin,
	Johannes Weiner, Vladimir Davydov
In-Reply-To: <CALvZod4oOddDvuvuXyp=p2Dq=h354a-D72daagfya_Ewp_ggSA@mail.gmail.com>

On 6/20/19 10:39 AM, Shakeel Butt wrote:
> On Thu, Jun 20, 2019 at 7:24 AM Waiman Long <longman@redhat.com> wrote:
>> On 6/19/19 7:48 PM, Shakeel Butt wrote:
>>> Hi Waiman,
>>>
>>> On Wed, Jun 19, 2019 at 10:16 AM Waiman Long <longman@redhat.com> wrote:
>>>> There are concerns about memory leaks from extensive use of memory
>>>> cgroups as each memory cgroup creates its own set of kmem caches. There
>>>> is a possiblity that the memcg kmem caches may remain even after the
>>>> memory cgroups have been offlined. Therefore, it will be useful to show
>>>> the status of each of memcg kmem caches.
>>>>
>>>> This patch introduces a new <debugfs>/memcg_slabinfo file which is
>>>> somewhat similar to /proc/slabinfo in format, but lists only information
>>>> about kmem caches that have child memcg kmem caches. Information
>>>> available in /proc/slabinfo are not repeated in memcg_slabinfo.
>>>>
>>>> A portion of a sample output of the file was:
>>>>
>>>>   # <name> <css_id[:dead]> <active_objs> <num_objs> <active_slabs> <num_slabs>
>>>>   rpc_inode_cache   root          13     51      1      1
>>>>   rpc_inode_cache     48           0      0      0      0
>>>>   fat_inode_cache   root           1     45      1      1
>>>>   fat_inode_cache     41           2     45      1      1
>>>>   xfs_inode         root         770    816     24     24
>>>>   xfs_inode           92          22     34      1      1
>>>>   xfs_inode           88:dead      1     34      1      1
>>>>   xfs_inode           89:dead     23     34      1      1
>>>>   xfs_inode           85           4     34      1      1
>>>>   xfs_inode           84           9     34      1      1
>>>>
>>>> The css id of the memcg is also listed. If a memcg is not online,
>>>> the tag ":dead" will be attached as shown above.
>>>>
>>>> Suggested-by: Shakeel Butt <shakeelb@google.com>
>>>> Signed-off-by: Waiman Long <longman@redhat.com>
>>>> ---
>>>>  mm/slab_common.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 57 insertions(+)
>>>>
>>>> diff --git a/mm/slab_common.c b/mm/slab_common.c
>>>> index 58251ba63e4a..2bca1558a722 100644
>>>> --- a/mm/slab_common.c
>>>> +++ b/mm/slab_common.c
>>>> @@ -17,6 +17,7 @@
>>>>  #include <linux/uaccess.h>
>>>>  #include <linux/seq_file.h>
>>>>  #include <linux/proc_fs.h>
>>>> +#include <linux/debugfs.h>
>>>>  #include <asm/cacheflush.h>
>>>>  #include <asm/tlbflush.h>
>>>>  #include <asm/page.h>
>>>> @@ -1498,6 +1499,62 @@ static int __init slab_proc_init(void)
>>>>         return 0;
>>>>  }
>>>>  module_init(slab_proc_init);
>>>> +
>>>> +#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_MEMCG_KMEM)
>>>> +/*
>>>> + * Display information about kmem caches that have child memcg caches.
>>>> + */
>>>> +static int memcg_slabinfo_show(struct seq_file *m, void *unused)
>>>> +{
>>>> +       struct kmem_cache *s, *c;
>>>> +       struct slabinfo sinfo;
>>>> +
>>>> +       mutex_lock(&slab_mutex);
>>> On large machines there can be thousands of memcgs and potentially
>>> each memcg can have hundreds of kmem caches. So, the slab_mutex can be
>>> held for a very long time.
>> But that is also what /proc/slabinfo does by doing mutex_lock() at
>> slab_start() and mutex_unlock() at slab_stop(). So the same problem will
>> happen when /proc/slabinfo is being read.
>>
>> When you are in a situation that reading /proc/slabinfo take a long time
>> because of the large number of memcg's, the system is in some kind of
>> trouble anyway. I am saying that we should not improve the scalability
>> of this patch. It is just that some nasty race conditions may pop up if
>> we release the lock and re-acquire it latter. That will greatly
>> complicate the code to handle all those edge cases.
>>
> We have been using that interface and implementation for couple of
> years and have not seen any race condition. However I am fine with
> what you have here for now. We can always come back if we think we
> need to improve it.
>
>>> Our internal implementation traverses the memcg tree and then
>>> traverses 'memcg->kmem_caches' within the slab_mutex (and
>>> cond_resched() after unlock).
>> For cgroup v1, the setting of the CONFIG_SLUB_DEBUG option will allow
>> you to iterate and display slabinfo just for that particular memcg. I am
>> thinking of extending the debug controller to do similar thing for
>> cgroup v2.
> I was also planning to look into that and it seems like you are
> already on it. Do CC me the patches.
>
Sure.


>>>> +       seq_puts(m, "# <name> <css_id[:dead]> <active_objs> <num_objs>");
>>>> +       seq_puts(m, " <active_slabs> <num_slabs>\n");
>>>> +       list_for_each_entry(s, &slab_root_caches, root_caches_node) {
>>>> +               /*
>>>> +                * Skip kmem caches that don't have any memcg children.
>>>> +                */
>>>> +               if (list_empty(&s->memcg_params.children))
>>>> +                       continue;
>>>> +
>>>> +               memset(&sinfo, 0, sizeof(sinfo));
>>>> +               get_slabinfo(s, &sinfo);
>>>> +               seq_printf(m, "%-17s root      %6lu %6lu %6lu %6lu\n",
>>>> +                          cache_name(s), sinfo.active_objs, sinfo.num_objs,
>>>> +                          sinfo.active_slabs, sinfo.num_slabs);
>>>> +
>>>> +               for_each_memcg_cache(c, s) {
>>>> +                       struct cgroup_subsys_state *css;
>>>> +                       char *dead = "";
>>>> +
>>>> +                       css = &c->memcg_params.memcg->css;
>>>> +                       if (!(css->flags & CSS_ONLINE))
>>>> +                               dead = ":dead";
>>> Please note that Roman's kmem cache reparenting patch series have made
>>> kmem caches of zombie memcgs a bit tricky. On memcg offlining the
>>> memcg kmem caches are reparented and the css->id can get recycled. So,
>>> we want to know that the a kmem cache is reparented and which memcg it
>>> belonged to initially. Determining if a kmem cache is reparented, we
>>> can store a flag on the kmem cache and for the previous memcg we can
>>> use fhandle. However to not make this more complicated, for now, we
>>> can just have the info that the kmem cache was reparented i.e. belongs
>>> to an offlined memcg.
>> I need to play with Roman's kmem cache reparenting patch a bit more to
>> see how to properly recognize a reparent'ed kmem cache. What I have
>> noticed is that the dead kmem caches that I saw at boot up were gone
>> after applying his patch. So that is a good thing.
>>
> By gone, do you mean the kmem cache got freed or the kmem cache is not
> part of online parent memcg and thus no more dead kmem cache?
I just look at the online flag of the memcg's css. All of them are
online when the iteration is being done after Roman's patch. I will
probably need to check if reparenting has happened.
>
>> For now, I think the current patch is good enough for its purpose. I may
>> send follow-up if I see something that can be improved.
>>
> I would like to see the recognition of reparent'ed kmem cache in this
> patch. However if others are ok with the current status of the patch
> then I will not stand in the way.

As I said, I will work on a follow-up patch to recognize reparenting.

Cheers,
Longman


^ permalink raw reply

* [PATCH RFC v1 2/7] serial: imx: set_termios(): factor-out 'ucr2' initial value
From: Sergey Organov @ 2019-06-20 14:47 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Pengutronix Kernel Team, NXP Linux Team, linux-arm-kernel,
	linux-serial, Uwe Kleine-König
In-Reply-To: <1561042073-617-1-git-send-email-sorganov@gmail.com>

Set common bits in a separate statement to make initialization
explicit and not repeat the common part.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 drivers/tty/serial/imx.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 1055124..87802fd 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1563,10 +1563,9 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	spin_lock_irqsave(&sport->port.lock, flags);
 
+	ucr2 = UCR2_SRST | UCR2_IRTS;
 	if ((termios->c_cflag & CSIZE) == CS8)
-		ucr2 = UCR2_WS | UCR2_SRST | UCR2_IRTS;
-	else
-		ucr2 = UCR2_SRST | UCR2_IRTS;
+		ucr2 |= UCR2_WS;
 
 	if (termios->c_cflag & CRTSCTS) {
 		if (sport->have_rtscts) {
-- 
2.10.0.1.g57b01a3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH RFC v1 1/7] serial: imx: fix locking in set_termios()
From: Sergey Organov @ 2019-06-20 14:47 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Pengutronix Kernel Team, NXP Linux Team, linux-arm-kernel,
	linux-serial, Uwe Kleine-König
In-Reply-To: <1561042073-617-1-git-send-email-sorganov@gmail.com>

imx_uart_set_termios() called imx_uart_rts_active(), or
imx_uart_rts_inactive() before taking port->port.lock.

As a consequence, sport->port.mctrl that these functions modify
could have been changed without holding port->port.lock.

Moved locking of port->port.lock above the calls to fix the issue.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 drivers/tty/serial/imx.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index dff75dc..1055124 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -383,6 +383,7 @@ static void imx_uart_ucrs_restore(struct imx_port *sport,
 }
 #endif
 
+/* called with port.lock taken and irqs caller dependent */
 static void imx_uart_rts_active(struct imx_port *sport, u32 *ucr2)
 {
 	*ucr2 &= ~(UCR2_CTSC | UCR2_CTS);
@@ -391,6 +392,7 @@ static void imx_uart_rts_active(struct imx_port *sport, u32 *ucr2)
 	mctrl_gpio_set(sport->gpios, sport->port.mctrl);
 }
 
+/* called with port.lock taken and irqs caller dependent */
 static void imx_uart_rts_inactive(struct imx_port *sport, u32 *ucr2)
 {
 	*ucr2 &= ~UCR2_CTSC;
@@ -400,6 +402,7 @@ static void imx_uart_rts_inactive(struct imx_port *sport, u32 *ucr2)
 	mctrl_gpio_set(sport->gpios, sport->port.mctrl);
 }
 
+/* called with port.lock taken and irqs caller dependent */
 static void imx_uart_rts_auto(struct imx_port *sport, u32 *ucr2)
 {
 	*ucr2 |= UCR2_CTSC;
@@ -1550,6 +1553,16 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 		old_csize = CS8;
 	}
 
+	del_timer_sync(&sport->timer);
+
+	/*
+	 * Ask the core to calculate the divisor for us.
+	 */
+	baud = uart_get_baud_rate(port, termios, old, 50, port->uartclk / 16);
+	quot = uart_get_divisor(port, baud);
+
+	spin_lock_irqsave(&sport->port.lock, flags);
+
 	if ((termios->c_cflag & CSIZE) == CS8)
 		ucr2 = UCR2_WS | UCR2_SRST | UCR2_IRTS;
 	else
@@ -1593,16 +1606,6 @@ imx_uart_set_termios(struct uart_port *port, struct ktermios *termios,
 			ucr2 |= UCR2_PROE;
 	}
 
-	del_timer_sync(&sport->timer);
-
-	/*
-	 * Ask the core to calculate the divisor for us.
-	 */
-	baud = uart_get_baud_rate(port, termios, old, 50, port->uartclk / 16);
-	quot = uart_get_divisor(port, baud);
-
-	spin_lock_irqsave(&sport->port.lock, flags);
-
 	sport->port.read_status_mask = 0;
 	if (termios->c_iflag & INPCK)
 		sport->port.read_status_mask |= (URXD_FRMERR | URXD_PRERR);
-- 
2.10.0.1.g57b01a3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [Qemu-devel] [PATCH v2 1/4] xen: Fix build with public headers
From: Anthony PERARD @ 2019-06-20 14:44 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: xen-devel, Paul Durrant, Stefano Stabellini, qemu-devel
In-Reply-To: <20190618121408.GM28525@redhat.com>

On Tue, Jun 18, 2019 at 01:14:08PM +0100, Daniel P. Berrangé wrote:
> On Tue, Jun 18, 2019 at 12:23:38PM +0100, Anthony PERARD wrote:
> > Following 37677d7db3 "Clean up a few header guard symbols", QEMU start
> > to fail to build:
> > 
> > In file included from ~/xen/tools/../tools/include/xen/io/blkif.h:31:0,
> >                  from ~/xen/tools/qemu-xen-dir/hw/block/xen_blkif.h:5,
> >                  from ~/xen/tools/qemu-xen-dir/hw/block/xen-block.c:22:
> > ~/xen/tools/../tools/include/xen/io/ring.h:68:0: error: "__CONST_RING_SIZE" redefined [-Werror]
> >  #define __CONST_RING_SIZE(_s, _sz) \
> > 
> > In file included from ~/xen/tools/qemu-xen-dir/hw/block/xen_blkif.h:4:0,
> >                  from ~/xen/tools/qemu-xen-dir/hw/block/xen-block.c:22:
> > ~/xen/tools/qemu-xen-dir/include/hw/xen/io/ring.h:66:0: note: this is the location of the previous definition
> >  #define __CONST_RING_SIZE(_s, _sz) \
> > 
> > The issue is that some public xen headers have been imported (by
> > f65eadb639 "xen: import ring.h from xen") but not all. With the change
> > in the guards symbole, the ring.h header start to be imported twice.
> 
> Ah, so the include/hw/xen/io/ring.h file in tree is a copy of
> /usr/include/xen/io/ring.h from xen-devel.  Previously both
> these used "#ifndef __XEN_PUBLIC_IO_RING_H__". After
> the header guard cleanup in 37677d7db3, our local copy used a
> different header guard from the installed copy & thus we're
> not protected from dual inclusion.
> 
> IMHO the right solutions here are either
> 
>  - Don't copy public Xen headers into our tree
>  - Keep our Xen header copies identical to the originals
> 
> Importing public headers and then changing them locally is the worst
> thing to do. With that in mind I think we should revert the part of
> commit 37677d7db3 that touched the imported Xen headers.

Yes, it's propably a better thing to do. So, I'm going to update the
series and do:
- revert part of 37677d7db3
- import the public headers that depends on ring.h. Or in other words,
  the one that describe an interface with a guest.
  I'll do some modification on the headers but only to remove the stuff
  that QEMU doesn't need (like how to make an hypercall).

Thanks,

-- 
Anthony PERARD


^ permalink raw reply

* [U-Boot] [PATCH 6/6 v3] configs: ls1028a: enable networking options in rds, qds defconfig
From: Alex Marginean @ 2019-06-20 14:48 UTC (permalink / raw)
  To: u-boot
In-Reply-To: <20190620144828.11899-1-alexandru.marginean@nxp.com>

Enables ethernet, MDIO, PHY drivers for LS1028A RDB and QDS.

Signed-off-by: Alex Marginean <alexm.osslist@gmail.com>
---
 configs/ls1028aqds_tfa_defconfig | 5 ++++-
 configs/ls1028ardb_tfa_defconfig | 2 ++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/configs/ls1028aqds_tfa_defconfig b/configs/ls1028aqds_tfa_defconfig
index 7982ce4157..2ea594c43a 100644
--- a/configs/ls1028aqds_tfa_defconfig
+++ b/configs/ls1028aqds_tfa_defconfig
@@ -41,10 +41,13 @@ CONFIG_SPI_FLASH_SPANSION=y
 CONFIG_SPI_FLASH_STMICRO=y
 # CONFIG_SPI_FLASH_USE_4K_SECTORS is not set
 CONFIG_PHYLIB=y
+CONFIG_PHY_AQUANTIA=y
 CONFIG_PHY_ATHEROS=y
+CONFIG_PHY_VITESSE=y
 CONFIG_DM_ETH=y
-CONFIG_PHY_GIGE=y
+CONFIG_DM_MDIO=y
 CONFIG_E1000=y
+CONFIG_FSL_ENETC=y
 CONFIG_PCI=y
 CONFIG_DM_PCI=y
 CONFIG_DM_PCI_COMPAT=y
diff --git a/configs/ls1028ardb_tfa_defconfig b/configs/ls1028ardb_tfa_defconfig
index c65e37df79..3f5bc2e139 100644
--- a/configs/ls1028ardb_tfa_defconfig
+++ b/configs/ls1028ardb_tfa_defconfig
@@ -43,8 +43,10 @@ CONFIG_SPI_FLASH_STMICRO=y
 CONFIG_PHYLIB=y
 CONFIG_PHY_ATHEROS=y
 CONFIG_DM_ETH=y
+CONFIG_DM_MDIO=y
 CONFIG_PHY_GIGE=y
 CONFIG_E1000=y
+CONFIG_FSL_ENETC=y
 CONFIG_PCI=y
 CONFIG_DM_PCI=y
 CONFIG_DM_PCI_COMPAT=y
-- 
2.17.1

^ permalink raw reply related


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.