Re: svn commit: r362829 - head/sys/compat/linuxkpi/common/src

2020-07-01 Thread Konstantin Belousov
On Wed, Jul 01, 2020 at 12:56:12PM +0200, Hans Petter Selasky wrote:
> On 2020-07-01 12:30, Konstantin Belousov wrote:
> > I see no point in repeating the same pfind/tdfind calls, better to convert
> > them to pget(), and have this code in one intended place.
> 
> I wonder if we can convert all cases in linux_current.c to use pget(). Could
> you have a look too?

Other uses in linux_current.c are not suitable for pget().  In case
linux_pid_task()/linux_get_pid_task() were passed a tid instead of pid,
current code returns lkpi_task for the specified thread.  In other words,
if using pget(), we would need to find the thread after the call, which
makes no sense.

On the other hand, there are at least two aspects that can be improved.
First, the functions are too similar to require separating body.  Second,
distinction between pid and tid is static and if we are passed pid, it
makes no sense to call tdfind() on it.

https://reviews.freebsd.org/D25534
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r362829 - head/sys/compat/linuxkpi/common/src

2020-07-01 Thread Hans Petter Selasky

On 2020-07-01 12:30, Konstantin Belousov wrote:

I see no point in repeating the same pfind/tdfind calls, better to convert
them to pget(), and have this code in one intended place.


I wonder if we can convert all cases in linux_current.c to use pget(). 
Could you have a look too?


--HPS
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r362829 - head/sys/compat/linuxkpi/common/src

2020-07-01 Thread Konstantin Belousov
On Wed, Jul 01, 2020 at 11:35:21AM +0200, Hans Petter Selasky wrote:
> On 2020-07-01 11:21, Konstantin Belousov wrote:
> > It should be expressed as pget(pid, 0); instead of duplicating.
> 
> Hi,
> 
> Currently the LinuxKPI style is to use tdfind() and pfind(). If you look at
> linux_current.c you see multiple uses of the exact same syntax.
> 
> Quickly looking at the pget() implementation, I see it doesn't expand to
> exactly tdfind() and pfind(). pget() uses pfind_tid() which looks overkill
> compared to tdfind(). tdfind() uses a hash-table lookup, while pfind_tid()
> doesn't  I'm confused.

It is trivial to change pget() to use tdfind(),
https://reviews.freebsd.org/D25532

I see no point in repeating the same pfind/tdfind calls, better to convert
them to pget(), and have this code in one intended place.




the sam
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r362829 - head/sys/compat/linuxkpi/common/src

2020-07-01 Thread Hans Petter Selasky

On 2020-07-01 11:21, Konstantin Belousov wrote:

It should be expressed as pget(pid, 0); instead of duplicating.


Hi,

Currently the LinuxKPI style is to use tdfind() and pfind(). If you look 
at linux_current.c you see multiple uses of the exact same syntax.


Quickly looking at the pget() implementation, I see it doesn't expand to 
exactly tdfind() and pfind(). pget() uses pfind_tid() which looks 
overkill compared to tdfind(). tdfind() uses a hash-table lookup, while 
pfind_tid() doesn't  I'm confused.


--HPS


___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r362829 - head/sys/compat/linuxkpi/common/src

2020-07-01 Thread Konstantin Belousov
On Wed, Jul 01, 2020 at 08:23:57AM +, Hans Petter Selasky wrote:
> Author: hselasky
> Date: Wed Jul  1 08:23:57 2020
> New Revision: 362829
> URL: https://svnweb.freebsd.org/changeset/base/362829
> 
> Log:
>   The "pid" field in the LinuxKPI task struct is typically set to the thread 
> ID
>   and not the process ID. Make sure the linux_task_exiting() function uses 
> tdfind()
>   to lookup the BSD procedure structure pointer by the "pid" field, and only
>   fallback to pfind() when no match is found! This makes linux_task_exiting()
>   in line with the rest of the code.
>   
>   Differential Revision: https://reviews.freebsd.org/D25509
>   Submitted by:   Greg V 
>   MFC after:  1 week
>   Sponsored by:   Mellanox Technologies
> 
> Modified:
>   head/sys/compat/linuxkpi/common/src/linux_current.c
> 
> Modified: head/sys/compat/linuxkpi/common/src/linux_current.c
> ==
> --- head/sys/compat/linuxkpi/common/src/linux_current.c   Wed Jul  1 
> 05:59:08 2020(r362828)
> +++ head/sys/compat/linuxkpi/common/src/linux_current.c   Wed Jul  1 
> 08:23:57 2020(r362829)
> @@ -219,11 +219,21 @@ linux_get_pid_task(pid_t pid)
>  bool
>  linux_task_exiting(struct task_struct *task)
>  {
> + struct thread *td;
>   struct proc *p;
>   bool ret;
>  
>   ret = false;
> - p = pfind(task->pid);
> +
> + /* try to find corresponding thread */
> + td = tdfind(task->pid, -1);
> + if (td != NULL) {
> + p = td->td_proc;
> + } else {
> + /* try to find corresponding procedure */
> + p = pfind(task->pid);
> + }
> +
>   if (p != NULL) {
>   if ((p->p_flag & P_WEXIT) != 0)
>   ret = true;
It should be expressed as pget(pid, 0); instead of duplicating.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


svn commit: r362829 - head/sys/compat/linuxkpi/common/src

2020-07-01 Thread Hans Petter Selasky
Author: hselasky
Date: Wed Jul  1 08:23:57 2020
New Revision: 362829
URL: https://svnweb.freebsd.org/changeset/base/362829

Log:
  The "pid" field in the LinuxKPI task struct is typically set to the thread ID
  and not the process ID. Make sure the linux_task_exiting() function uses 
tdfind()
  to lookup the BSD procedure structure pointer by the "pid" field, and only
  fallback to pfind() when no match is found! This makes linux_task_exiting()
  in line with the rest of the code.
  
  Differential Revision: https://reviews.freebsd.org/D25509
  Submitted by: Greg V 
  MFC after:1 week
  Sponsored by: Mellanox Technologies

Modified:
  head/sys/compat/linuxkpi/common/src/linux_current.c

Modified: head/sys/compat/linuxkpi/common/src/linux_current.c
==
--- head/sys/compat/linuxkpi/common/src/linux_current.c Wed Jul  1 05:59:08 
2020(r362828)
+++ head/sys/compat/linuxkpi/common/src/linux_current.c Wed Jul  1 08:23:57 
2020(r362829)
@@ -219,11 +219,21 @@ linux_get_pid_task(pid_t pid)
 bool
 linux_task_exiting(struct task_struct *task)
 {
+   struct thread *td;
struct proc *p;
bool ret;
 
ret = false;
-   p = pfind(task->pid);
+
+   /* try to find corresponding thread */
+   td = tdfind(task->pid, -1);
+   if (td != NULL) {
+   p = td->td_proc;
+   } else {
+   /* try to find corresponding procedure */
+   p = pfind(task->pid);
+   }
+
if (p != NULL) {
if ((p->p_flag & P_WEXIT) != 0)
ret = true;
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"